🚀 fanquake merged a pull request: "depends: Fetch miniupnpc sources from an alternative website"
(https://github.com/bitcoin/bitcoin/pull/30151)
(https://github.com/bitcoin/bitcoin/pull/30151)
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611265859)
OHH, my thinking was that it's a WIN32 API function, not a network function. So i thought `SysErrorString` would be correct. But i think you're right. "SysError" is more like "posix errno emulation error". Which is not what is needed here.
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611265859)
OHH, my thinking was that it's a WIN32 API function, not a network function. So i thought `SysErrorString` would be correct. But i think you're right. "SysError" is more like "posix errno emulation error". Which is not what is needed here.
👍 t-bast approved a pull request: "policy: restrict all TRUC (v3) transactions to 10kvB"
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2073292210)
ACK https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2073292210)
ACK https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11
💬 pablomartin4btc commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#discussion_r1611272235)
done.
(https://github.com/bitcoin-core/gui/pull/815#discussion_r1611272235)
done.
💬 fanquake commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2126564542)
Backported to 27.x in #30092.
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2126564542)
Backported to 27.x in #30092.
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611280293)
> That said, this Windows code is a lot simpler than the Linux stuff above (ducks...).
You mean that centrally-controlled proprietary OSes sometimes have well-documented API's that consider use-cases, while FOSS often uses haphazard grabbag API's that were grown in accordance with one tool (where everyone is expected to parse text output of-)... or some custom library, with equally confusing interface. you can just say that out loud here you know... 😓
Standards like RFCs, and POSIX (ignori
...
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611280293)
> That said, this Windows code is a lot simpler than the Linux stuff above (ducks...).
You mean that centrally-controlled proprietary OSes sometimes have well-documented API's that consider use-cases, while FOSS often uses haphazard grabbag API's that were grown in accordance with one tool (where everyone is expected to parse text output of-)... or some custom library, with equally confusing interface. you can just say that out loud here you know... 😓
Standards like RFCs, and POSIX (ignori
...
💬 virtu commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#issuecomment-2126596267)
ACK [1e54d61](https://github.com/bitcoin/bitcoin/commit/1e54d61c4698debf3329d1960e06078ccbf8063c)
Successfully re-ran all previously mentioned tests.
(https://github.com/bitcoin/bitcoin/pull/30062#issuecomment-2126596267)
ACK [1e54d61](https://github.com/bitcoin/bitcoin/commit/1e54d61c4698debf3329d1960e06078ccbf8063c)
Successfully re-ran all previously mentioned tests.
💬 maflcko commented on issue "ci: Enable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2126604835)
An alternative might be to try to set `--privileged` on the GHA docker, but I am not sure if this is possible.
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2126604835)
An alternative might be to try to set `--privileged` on the GHA docker, but I am not sure if this is possible.
💬 fanquake commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2126616244)
> CI should be bumped as well, if this is taken out of draft.
Added a (WIP) commit to bump a couple CIs to 13.2.0. Also bumped the time-machine further, now that we've upstreamed GCC 13.3.0: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=750148ce1ea6c65a7c14424546db0078161f7e17.
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2126616244)
> CI should be bumped as well, if this is taken out of draft.
Added a (WIP) commit to bump a couple CIs to 13.2.0. Also bumped the time-machine further, now that we've upstreamed GCC 13.3.0: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=750148ce1ea6c65a7c14424546db0078161f7e17.
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1611332854)
Yes, indeed! Fixed, thanks!
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1611332854)
Yes, indeed! Fixed, thanks!
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2126637345)
`9a158da46c...ecba2fbd20`: rebase due to conflicts, thanks, @jonatack!
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2126637345)
`9a158da46c...ecba2fbd20`: rebase due to conflicts, thanks, @jonatack!
👋 vasild's pull request is ready for review: "test: add end-to-end tests for CConnman and PeerManager"
(https://github.com/bitcoin/bitcoin/pull/26812)
(https://github.com/bitcoin/bitcoin/pull/26812)
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611345455)
Maybe also check that the result is not 0.0.0.0 (::/0 for IPv6 below). It's not entirely clear to me from the documentation if that can realistically happen, but doesn't hurt to check either.
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611345455)
Maybe also check that the result is not 0.0.0.0 (::/0 for IPv6 below). It's not entirely clear to me from the documentation if that can realistically happen, but doesn't hurt to check either.
💬 vasild commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1611353096)
Makes sense (also for session destroy).
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1611353096)
Makes sense (also for session destroy).
👍 fanquake approved a pull request: "contrib: Renew Windows code signing certificate"
(https://github.com/bitcoin/bitcoin/pull/30149#pullrequestreview-2073430081)
ACK 9f4ff1e9659597307f62510f1885ad8da3a1d9a3
(https://github.com/bitcoin/bitcoin/pull/30149#pullrequestreview-2073430081)
ACK 9f4ff1e9659597307f62510f1885ad8da3a1d9a3
💬 vasild commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1611363214)
Note that for transient I2P sessions (when `-i2pacceptincoming=0`) this would be printed for every I2P peer connect and disconnect even if I2P logging is switched off.
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1611363214)
Note that for transient I2P sessions (when `-i2pacceptincoming=0`) this would be printed for every I2P peer connect and disconnect even if I2P logging is switched off.
💬 vasild commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2126693545)
`6bfda84b92...b5189f543c`: rebase due to conflicts (part of this PR was merged via https://github.com/bitcoin/bitcoin/pull/29421)
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2126693545)
`6bfda84b92...b5189f543c`: rebase due to conflicts (part of this PR was merged via https://github.com/bitcoin/bitcoin/pull/29421)
👋 vasild's pull request is ready for review: "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'"
(https://github.com/bitcoin/bitcoin/pull/29418)
(https://github.com/bitcoin/bitcoin/pull/29418)
💬 maflcko commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#issuecomment-2126708166)
> (reposting using the "review changes" button to appease the bot, it sent me several "request review" notifications in the last ten minutes :)
Not sure why that would happen. The bot should not react on new review comments after someone else's ACK. (c.f. https://github.com/maflcko/DrahtBot/blob/main/webhook_features/src/features/summary_comment.rs#L308) Looking at other pull requests, this seems to be working correctly, so my guess is that this is another GitHub metatdata corruption bug, whi
...
(https://github.com/bitcoin/bitcoin/pull/29833#issuecomment-2126708166)
> (reposting using the "review changes" button to appease the bot, it sent me several "request review" notifications in the last ten minutes :)
Not sure why that would happen. The bot should not react on new review comments after someone else's ACK. (c.f. https://github.com/maflcko/DrahtBot/blob/main/webhook_features/src/features/summary_comment.rs#L308) Looking at other pull requests, this seems to be working correctly, so my guess is that this is another GitHub metatdata corruption bug, whi
...
💬 hebasto commented on pull request "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP":
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2126713499)
> I've noticed two issues in the "tidy" job so far:
>
> 1. `Cannot open mapping file '/ci_container_base/ci/scratch/build-/contrib/devtools/iwyu/bitcoin.core.imp': No such file or directory.`
The recent push includes commits from https://github.com/hebasto/bitcoin/pull/205, which fixes the point 1.
> 2. Missed disabling `EXPORT_COMPILE_COMMANDS` property for targets in the `crc32c` subtree.
The point 2 has been fixed already.
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2126713499)
> I've noticed two issues in the "tidy" job so far:
>
> 1. `Cannot open mapping file '/ci_container_base/ci/scratch/build-/contrib/devtools/iwyu/bitcoin.core.imp': No such file or directory.`
The recent push includes commits from https://github.com/hebasto/bitcoin/pull/205, which fixes the point 1.
> 2. Missed disabling `EXPORT_COMPILE_COMMANDS` property for targets in the `crc32c` subtree.
The point 2 has been fixed already.