💬 hebasto commented on pull request "kernel: Remove batchpriority from kernel library":
(https://github.com/bitcoin/bitcoin/pull/30083#issuecomment-2121359133)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/202.
(https://github.com/bitcoin/bitcoin/pull/30083#issuecomment-2121359133)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/202.
💬 hebasto commented on pull request "build: swap cctools otool for llvm-objdump":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2121359541)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/202.
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2121359541)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/202.
💬 hebasto commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-2121360201)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/202.
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-2121360201)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/202.
💬 hebasto commented on issue "CMake-based build system tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-2121368098)
The main part of the https://github.com/bitcoin/bitcoin/issues/28607#issue-1929913410 has all checkboxes checked.
Thanks to all reviewers and testers!
Keep working :)
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-2121368098)
The main part of the https://github.com/bitcoin/bitcoin/issues/28607#issue-1929913410 has all checkboxes checked.
Thanks to all reviewers and testers!
Keep working :)
💬 sipa commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2121393948)
Trying this with `-pcp=1` on my home internet connection:
```
2024-05-20T23:28:41.625432Z [net] pcp: gateway [IPv4]: 192.168.1.1
2024-05-20T23:28:41.625469Z [net] pcp: Requesting port mapping for addr 0.0.0.0 port 8333 from gateway 192.168.1.1
2024-05-20T23:28:41.625529Z [net] pcp: Internal address after connect: 192.168.1.254
2024-05-20T23:28:41.626388Z [net] pcp: Received response of 8 bytes: 008100010046cfb7
2024-05-20T23:28:41.626404Z [net:warning] pcp: Response too small
2024-05-20T2
...
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2121393948)
Trying this with `-pcp=1` on my home internet connection:
```
2024-05-20T23:28:41.625432Z [net] pcp: gateway [IPv4]: 192.168.1.1
2024-05-20T23:28:41.625469Z [net] pcp: Requesting port mapping for addr 0.0.0.0 port 8333 from gateway 192.168.1.1
2024-05-20T23:28:41.625529Z [net] pcp: Internal address after connect: 192.168.1.254
2024-05-20T23:28:41.626388Z [net] pcp: Received response of 8 bytes: 008100010046cfb7
2024-05-20T23:28:41.626404Z [net:warning] pcp: Response too small
2024-05-20T2
...
💬 kristapsk commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1607472804)
@ajtowns Good idea, can add test simple way without adding much more code. 84e03ab48e0c832ab2c0d8e6babb6563a0ee92dc
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1607472804)
@ajtowns Good idea, can add test simple way without adding much more code. 84e03ab48e0c832ab2c0d8e6babb6563a0ee92dc
📝 maflcko opened a pull request: "ci: Add mising -Wno-error=maybe-uninitialized to armhf task"
(https://github.com/bitcoin/bitcoin/pull/30144)
(https://github.com/bitcoin/bitcoin/pull/30144)
💬 maflcko commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#discussion_r1607698133)
Fixed in https://github.com/bitcoin/bitcoin/pull/30144
(https://github.com/bitcoin/bitcoin/pull/29521#discussion_r1607698133)
Fixed in https://github.com/bitcoin/bitcoin/pull/30144
💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2121826810)
The CI failure can be ignored during review. It is fixed in https://github.com/bitcoin/bitcoin/pull/30144
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2121826810)
The CI failure can be ignored during review. It is fixed in https://github.com/bitcoin/bitcoin/pull/30144
💬 maflcko commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1607712689)
style-wise, I wonder if it makes sense to call wait_until and find_conn 6 times, when all logic could be put into a single function, which will be passed as a `lambda` to wait_until once.
I guess this would mean slightly cleaner code, but would make it harder to see which part of the single function returned `False` when a failure happens. Though, debug logs could be added to help with that, if needed.
What do you think?
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1607712689)
style-wise, I wonder if it makes sense to call wait_until and find_conn 6 times, when all logic could be put into a single function, which will be passed as a `lambda` to wait_until once.
I guess this would mean slightly cleaner code, but would make it harder to see which part of the single function returned `False` when a failure happens. Though, debug logs could be added to help with that, if needed.
What do you think?
💬 maflcko commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1607713755)
nit: Could add a comment to where the ID is set, to clarify that it is now required for `connect_nodes` to work?
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1607713755)
nit: Could add a comment to where the ID is set, to clarify that it is now required for `connect_nodes` to work?
💬 maflcko commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#issuecomment-2121847793)
> This approach is fragile, as any of the peers involved in the process can drop, lose, or
> create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely
> even when the peers in question were connected successfully.
I'd say the tests should avoid racy code and deterministically execute the test code line-by-line, but enforcing this in `connect_nodes` is the wrong approach. So Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/30118#issuecomment-2121847793)
> This approach is fragile, as any of the peers involved in the process can drop, lose, or
> create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely
> even when the peers in question were connected successfully.
I'd say the tests should avoid racy code and deterministically execute the test code line-by-line, but enforcing this in `connect_nodes` is the wrong approach. So Concept ACK.
💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1607767150)
Updated to remove the `extra` argument.
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1607767150)
Updated to remove the `extra` argument.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1607790271)
Hmm, maybe we can keep track of whether we have have successfully managed to connect to at least one `.onion` address via the `-proxy`, then we can assume that is a Tor proxy? How does that sound?
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1607790271)
Hmm, maybe we can keep track of whether we have have successfully managed to connect to at least one `.onion` address via the `-proxy`, then we can assume that is a Tor proxy? How does that sound?
💬 virtu commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2121965779)
LGTM! ACK [8109319](https://github.com/bitcoin/bitcoin/commit/8109319e102c41d3aeed0ecfbc3a0e23b7fea807)
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2121965779)
LGTM! ACK [8109319](https://github.com/bitcoin/bitcoin/commit/8109319e102c41d3aeed0ecfbc3a0e23b7fea807)
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2121977231)
> Trying this with -pcp=1 on my home internet connection:
Thanks for testing!
The received packet is not a valid PCP response (too short). But interpreting it as one anyway, the version byte is 0x00, result code is 0x01 (UNSUPP_VERSION). So you might have one of those routers that supports NAT-PMP only.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2121977231)
> Trying this with -pcp=1 on my home internet connection:
Thanks for testing!
The received packet is not a valid PCP response (too short). But interpreting it as one anyway, the version byte is 0x00, result code is 0x01 (UNSUPP_VERSION). So you might have one of those routers that supports NAT-PMP only.
💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2122051145)
re-ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2122051145)
re-ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1607890616)
I like this, no need to introduce all this error handling infrastructure for a debug-only option. Applied.
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1607890616)
I like this, no need to introduce all this error handling infrastructure for a debug-only option. Applied.
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2122100379)
Updated 79c9c55b27bd113885da2c36ad76e5ed145027b3 -> 4a1df97a016935348e50f384ed52a6671990835f ([noGlobalScriptCache_0](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_0) -> [noGlobalScriptCache_1](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_0..noGlobalScriptCache_1))
* Addressed @theuni's [comment](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1607175446), made `C
...
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2122100379)
Updated 79c9c55b27bd113885da2c36ad76e5ed145027b3 -> 4a1df97a016935348e50f384ed52a6671990835f ([noGlobalScriptCache_0](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_0) -> [noGlobalScriptCache_1](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_0..noGlobalScriptCache_1))
* Addressed @theuni's [comment](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1607175446), made `C
...
🚀 fanquake merged a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606)
(https://github.com/bitcoin/bitcoin/pull/26606)