💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1263163161)
done
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1263163161)
done
💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1263163190)
done
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1263163190)
done
✅ JayBitron closed a pull request: "exclude ipc scheme from port check"
(https://github.com/bitcoin/bitcoin/pull/28020)
(https://github.com/bitcoin/bitcoin/pull/28020)
💬 MarcoFalke commented on pull request "refactor: Continue moving application data from CNode to Peer":
(https://github.com/bitcoin/bitcoin/pull/26621#issuecomment-1635315780)
Needs rebase if still relevant
(https://github.com/bitcoin/bitcoin/pull/26621#issuecomment-1635315780)
Needs rebase if still relevant
🤔 MarcoFalke reviewed a pull request: "test: add end-to-end tests for CConnman and PeerManager"
(https://github.com/bitcoin/bitcoin/pull/26812#pullrequestreview-1529671710)
Not sure about changing the framework.
Also, needs rebase if still relevant.
(https://github.com/bitcoin/bitcoin/pull/26812#pullrequestreview-1529671710)
Not sure about changing the framework.
Also, needs rebase if still relevant.
💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1263332063)
Not sure about changing this. It just makes it harder to use. If you prefer to not have an exception, this can use an `Assert`? Also, in the commit description, it would be good to refer to documentation why it is "not allowed", and/or include an example of what happens.
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1263332063)
Not sure about changing this. It just makes it harder to use. If you prefer to not have an exception, this can use an `Assert`? Also, in the commit description, it would be good to refer to documentation why it is "not allowed", and/or include an example of what happens.
💬 MarcoFalke commented on pull request "net: Use serialization parameters for CAddress serialization":
(https://github.com/bitcoin/bitcoin/pull/25284#issuecomment-1635587006)
rebased
(https://github.com/bitcoin/bitcoin/pull/25284#issuecomment-1635587006)
rebased
💬 dergoegge commented on pull request "refactor: Continue moving application data from CNode to Peer":
(https://github.com/bitcoin/bitcoin/pull/26621#issuecomment-1635588195)
> Needs rebase if still relevant
Thanks, rebased
(https://github.com/bitcoin/bitcoin/pull/26621#issuecomment-1635588195)
> Needs rebase if still relevant
Thanks, rebased
💬 fanquake commented on pull request "ci: build Valgrind (3.21) from source":
(https://github.com/bitcoin/bitcoin/pull/27992#issuecomment-1635607860)
I'll follow up with just adding more suppressions until things work on aarch64.
(https://github.com/bitcoin/bitcoin/pull/27992#issuecomment-1635607860)
I'll follow up with just adding more suppressions until things work on aarch64.
✅ fanquake closed a pull request: "ci: build Valgrind (3.21) from source"
(https://github.com/bitcoin/bitcoin/pull/27992)
(https://github.com/bitcoin/bitcoin/pull/27992)
💬 fanquake commented on pull request "ci: Add missing -O2 to valgrind tasks":
(https://github.com/bitcoin/bitcoin/pull/28071#issuecomment-1635610985)
> Right. Probably not worth to spend time here,
I'll follow up with this, because it certainly shouldn't be broken, and if it is, it's not specific to valgrind etc.
(https://github.com/bitcoin/bitcoin/pull/28071#issuecomment-1635610985)
> Right. Probably not worth to spend time here,
I'll follow up with this, because it certainly shouldn't be broken, and if it is, it's not specific to valgrind etc.
🚀 fanquake merged a pull request: "ci: Add missing -O2 to valgrind tasks"
(https://github.com/bitcoin/bitcoin/pull/28071)
(https://github.com/bitcoin/bitcoin/pull/28071)
👍 hebasto approved a pull request: "kernel: Remove StartShutdown calls from validation code"
(https://github.com/bitcoin/bitcoin/pull/28048#pullrequestreview-1530008568)
ACK 31eca93a9eb8e54f856d3f558aa3c831ef181d37, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/28048#pullrequestreview-1530008568)
ACK 31eca93a9eb8e54f856d3f558aa3c831ef181d37, I have reviewed the code and it looks OK.
💬 hebasto commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1263547071)
This requires to `#include "kernel/notifications_interface.h"`, no?
(https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1263547071)
This requires to `#include "kernel/notifications_interface.h"`, no?
💬 MarcoFalke commented on pull request "ci: build Valgrind (3.21) from source":
(https://github.com/bitcoin/bitcoin/pull/27992#issuecomment-1635633574)
I haven't tried, but is this still an issue on aarch64 on current master?
(https://github.com/bitcoin/bitcoin/pull/27992#issuecomment-1635633574)
I haven't tried, but is this still an issue on aarch64 on current master?
💬 MarcoFalke commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1635635435)
I haven't tried, but an alternative to using `clang` would be to try to set `-O1` for `gcc`, but I am not sure on how to do this cleanly?
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1635635435)
I haven't tried, but an alternative to using `clang` would be to try to set `-O1` for `gcc`, but I am not sure on how to do this cleanly?
💬 hebasto commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#issuecomment-1635642358)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27866#issuecomment-1635642358)
Concept ACK.
📝 MarcoFalke opened a pull request: "util: Remove DirIsWritable, GetUniquePath "
(https://github.com/bitcoin/bitcoin/pull/28075)
`GetUniquePath` is only used in tests and in `DirIsWritable`. The check by `DirIsWritable` is redundant with the check done in `LockDirectory`.
Fix the redundancy by removing everything, except `LockDirectory`.
(https://github.com/bitcoin/bitcoin/pull/28075)
`GetUniquePath` is only used in tests and in `DirIsWritable`. The check by `DirIsWritable` is redundant with the check done in `LockDirectory`.
Fix the redundancy by removing everything, except `LockDirectory`.
💬 stickies-v commented on pull request "rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1263578714)
Are you still going to make these changes?
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1263578714)
Are you still going to make these changes?
💬 fanquake commented on pull request "util: Remove DirIsWritable, GetUniquePath":
(https://github.com/bitcoin/bitcoin/pull/28075#issuecomment-1635664090)
```bash
make[2]: Entering directory '/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-arm-linux-gnueabihf/src'
/usr/bin/ccache arm-linux-gnueabihf-g++ -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config -fmacro-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-arm-linux-gnueabihf=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -DHAVE_BUILD_INFO -D_FILE_OFFSET_BITS=64 -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -I./leveldb/include -isystem /tmp/cir
...
(https://github.com/bitcoin/bitcoin/pull/28075#issuecomment-1635664090)
```bash
make[2]: Entering directory '/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-arm-linux-gnueabihf/src'
/usr/bin/ccache arm-linux-gnueabihf-g++ -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config -fmacro-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-arm-linux-gnueabihf=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -DHAVE_BUILD_INFO -D_FILE_OFFSET_BITS=64 -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -I./leveldb/include -isystem /tmp/cir
...