🤔 delta1 reviewed a pull request: "chore: remove redundant word"
(https://github.com/bitcoin/bitcoin/pull/31858#pullrequestreview-2615179552)
ACK https://github.com/bitcoin/bitcoin/commit/4c62b37fcd23e94ea482efc5974acc2560f57f5f
(https://github.com/bitcoin/bitcoin/pull/31858#pullrequestreview-2615179552)
ACK https://github.com/bitcoin/bitcoin/commit/4c62b37fcd23e94ea482efc5974acc2560f57f5f
🚀 fanquake merged a pull request: "chore: remove redundant word"
(https://github.com/bitcoin/bitcoin/pull/31858)
(https://github.com/bitcoin/bitcoin/pull/31858)
💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954615547)
See https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954576364
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954615547)
See https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954576364
💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954626155)
Thanks, that indeed looks simpler.
I also think clients should handle timeouts themselves. For `bitcoin-cli` we have `-rpcclienttimeout=0`, other RPC clients presumably have their own timeout mechanisms, and IPC clients can do the same.
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954626155)
Thanks, that indeed looks simpler.
I also think clients should handle timeouts themselves. For `bitcoin-cli` we have `-rpcclienttimeout=0`, other RPC clients presumably have their own timeout mechanisms, and IPC clients can do the same.
👍 hodlinator approved a pull request: "net: reduce CAddress usage to CService or CNetAddr"
(https://github.com/bitcoin/bitcoin/pull/31854#pullrequestreview-2615228759)
ACK cd4bfaee103591f212b88afd17969c16c1056eb6
### Concept
Defers "faking" of `CAddress` by initializing `CAddress::nServices = NODE_NONE` when all we actually have is a `CService` (network address+port), to all the way down to the construction of `CNode`:
https://github.com/bitcoin/bitcoin/blob/cd4bfaee103591f212b88afd17969c16c1056eb6/src/net.cpp#L1831-L1833
### Type safety sanity check
```mermaid
graph LR;
CService-->CNetAddr;
CAddress-->CService;
```
`CAddress`,
...
(https://github.com/bitcoin/bitcoin/pull/31854#pullrequestreview-2615228759)
ACK cd4bfaee103591f212b88afd17969c16c1056eb6
### Concept
Defers "faking" of `CAddress` by initializing `CAddress::nServices = NODE_NONE` when all we actually have is a `CService` (network address+port), to all the way down to the construction of `CNode`:
https://github.com/bitcoin/bitcoin/blob/cd4bfaee103591f212b88afd17969c16c1056eb6/src/net.cpp#L1831-L1833
### Type safety sanity check
```mermaid
graph LR;
CService-->CNetAddr;
CAddress-->CService;
```
`CAddress`,
...
💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2656828338)
I mixed up the commits in my last push, will fix...
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2656828338)
I mixed up the commits in my last push, will fix...
💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1954644132)
> Blah, ok, sorry. I've been messing around with this. Final answer:
>
> * Let's add a `SANITIZER_LIBS` var
>
> * add BOTH `LINK_LIBRARIES` and `LDFLAGS` to check_cxx_source_compiles_with_flags
>
> * pass them both for the sanitizers
>
> * update oss-fuzz to use `SANITIZER_LIBS` instead.
>
>
> That way our stuff is well-defined and makes sense and we're not coding around oss-fuzz, but we still expose the option we need there.
Reworked. A new `FUZZ_LIBS` variab
...
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1954644132)
> Blah, ok, sorry. I've been messing around with this. Final answer:
>
> * Let's add a `SANITIZER_LIBS` var
>
> * add BOTH `LINK_LIBRARIES` and `LDFLAGS` to check_cxx_source_compiles_with_flags
>
> * pass them both for the sanitizers
>
> * update oss-fuzz to use `SANITIZER_LIBS` instead.
>
>
> That way our stuff is well-defined and makes sense and we're not coding around oss-fuzz, but we still expose the option we need there.
Reworked. A new `FUZZ_LIBS` variab
...
💬 InnDe commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656840009)
Ok, thanks for the clarification.
hope someone figure this out
On Thu, 13 Feb 2025 at 17:30, Pieter Wuille ***@***.***>
wrote:
> Ok, well, if the password you set when creating the wallet in the GUI is \
> \ \ \ \ \ \ \ \ \ \ \, then the command in the debug console should be walletpassphrase
> '\ \ \ \ \ \ \ \ \ \ \ \' 800, or walletpassphrase "\\ \\ \\ \\ \\ \\ \\
> \\ \\ \\ \\ \\" 800, because of backslash quoting.
>
> But I cannot explain why you're not seeing any output at all.
...
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656840009)
Ok, thanks for the clarification.
hope someone figure this out
On Thu, 13 Feb 2025 at 17:30, Pieter Wuille ***@***.***>
wrote:
> Ok, well, if the password you set when creating the wallet in the GUI is \
> \ \ \ \ \ \ \ \ \ \ \, then the command in the debug console should be walletpassphrase
> '\ \ \ \ \ \ \ \ \ \ \ \' 800, or walletpassphrase "\\ \\ \\ \\ \\ \\ \\
> \\ \\ \\ \\ \\" 800, because of backslash quoting.
>
> But I cannot explain why you're not seeing any output at all.
...
💬 laanwj commented on pull request "test, tracing: don't use problematic `bpf_usdt_readarg_p()`":
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2656847216)
> bpf_usdt_readarg_p() is a helper that does this, but apparently not safely enough
i looked into it a bit; the implementation of `bpf_usdt_readarg_p` is here:
https://github.com/iovisor/bcc/blob/master/src/cc/frontends/clang/b_frontend_action.cc#L1270
It's a macro implemented in the clang frontend:
```c++
} else if (Decl->getName() == "bpf_usdt_readarg_p") {
text = "({ u64 __addr = 0x0; ";
text += "_bpf_readarg_" + current_fn_ + "_" + args[0] + "(" +
...
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2656847216)
> bpf_usdt_readarg_p() is a helper that does this, but apparently not safely enough
i looked into it a bit; the implementation of `bpf_usdt_readarg_p` is here:
https://github.com/iovisor/bcc/blob/master/src/cc/frontends/clang/b_frontend_action.cc#L1270
It's a macro implemented in the clang frontend:
```c++
} else if (Decl->getName() == "bpf_usdt_readarg_p") {
text = "({ u64 __addr = 0x0; ";
text += "_bpf_readarg_" + current_fn_ + "_" + args[0] + "(" +
...
💬 jsarenik commented on issue "Memory-only wallet (for faucets)":
(https://github.com/bitcoin/bitcoin/issues/31816#issuecomment-2656848957)
@maflcko thanks for the idea with ``removeprunedfunds but I just tried it and it did not after removing almost all of the already-mined transactions (the list I got from listtransactions RPC with some postprocessing).
(https://github.com/bitcoin/bitcoin/issues/31816#issuecomment-2656848957)
@maflcko thanks for the idea with ``removeprunedfunds but I just tried it and it did not after removing almost all of the already-mined transactions (the list I got from listtransactions RPC with some postprocessing).
💬 jsarenik commented on issue "Memory-only wallet (for faucets)":
(https://github.com/bitcoin/bitcoin/issues/31816#issuecomment-2656857139)
@maflcko thanks for the idea with ``removeprunedfunds but I just tried it and it did not after removing almost all of the already-mined transactions (the list I got from listtransactions RPC with some postprocessing).
I think the database file would need to be vacuumed.
(https://github.com/bitcoin/bitcoin/issues/31816#issuecomment-2656857139)
@maflcko thanks for the idea with ``removeprunedfunds but I just tried it and it did not after removing almost all of the already-mined transactions (the list I got from listtransactions RPC with some postprocessing).
I think the database file would need to be vacuumed.
💬 laanwj commented on pull request "test, tracing: don't use problematic `bpf_usdt_readarg_p()`":
(https://github.com/bitcoin/bitcoin/pull/31848#discussion_r1954661905)
Using `bpf_probe_read_user_str` here at least makes sure that only until the zero-terminator is read and not the entire buffer.
(https://github.com/bitcoin/bitcoin/pull/31848#discussion_r1954661905)
Using `bpf_probe_read_user_str` here at least makes sure that only until the zero-terminator is read and not the entire buffer.
✅ jsarenik closed an issue: "Memory-only wallet (for faucets)"
(https://github.com/bitcoin/bitcoin/issues/31816)
(https://github.com/bitcoin/bitcoin/issues/31816)
💬 jsarenik commented on issue "Memory-only wallet (for faucets)":
(https://github.com/bitcoin/bitcoin/issues/31816#issuecomment-2656886387)
Yes, @maflcko , it works well, I just need to unload the wallet, `echo 'VACUUM;' | sqlite3 wallet dat` and load it back.
I think the feature suggestion in this issue still makes sense though.
(https://github.com/bitcoin/bitcoin/issues/31816#issuecomment-2656886387)
Yes, @maflcko , it works well, I just need to unload the wallet, `echo 'VACUUM;' | sqlite3 wallet dat` and load it back.
I think the feature suggestion in this issue still makes sense though.
💬 sipa commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656903312)
Without the ability to reproduce the problem, there is little that can be done.
Can you tell us a bit more about your set-up? The issue says you compiled it from source? Which compiler?
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656903312)
Without the ability to reproduce the problem, there is little that can be done.
Can you tell us a bit more about your set-up? The issue says you compiled it from source? Which compiler?
🤔 stickies-v reviewed a pull request: "Add -pruneduringinit option to temporarily use another prune target during IBD"
(https://github.com/bitcoin/bitcoin/pull/31845#pullrequestreview-2615366818)
> Can't we just do an IBD without pruning
Disabling pruning is only possible on devices with sufficient disk space, but I agree that a simple restart seems like an easy enough workaround if benchmarks indicate that this a worthwhile performance improvement:
```
bitcoind --prune=4000 --stopatheight={current_tip} && bitcoind --prune=300
```
Approach NACK, I don't think this needs a separate startup option. If benchmarks agree, I think a PR that improves the `--prune` documentation would
...
(https://github.com/bitcoin/bitcoin/pull/31845#pullrequestreview-2615366818)
> Can't we just do an IBD without pruning
Disabling pruning is only possible on devices with sufficient disk space, but I agree that a simple restart seems like an easy enough workaround if benchmarks indicate that this a worthwhile performance improvement:
```
bitcoind --prune=4000 --stopatheight={current_tip} && bitcoind --prune=300
```
Approach NACK, I don't think this needs a separate startup option. If benchmarks agree, I think a PR that improves the `--prune` documentation would
...
💬 InnDe commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656918683)
I install the software from www.bitcoin.org
On Thu, 13 Feb 2025 at 18:10, Pieter Wuille ***@***.***>
wrote:
> Without the ability to reproduce the problem, there is little that can be
> done.
>
> Can you tell us a bit more about your set-up? The issue says you compiled
> it from source? Which compiler?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656903312>,
> or unsubscribe
> <https://github.com/notific
...
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656918683)
I install the software from www.bitcoin.org
On Thu, 13 Feb 2025 at 18:10, Pieter Wuille ***@***.***>
wrote:
> Without the ability to reproduce the problem, there is little that can be
> done.
>
> Can you tell us a bit more about your set-up? The issue says you compiled
> it from source? Which compiler?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656903312>,
> or unsubscribe
> <https://github.com/notific
...
💬 hebasto commented on pull request "depends: add missing Darwin objcopy":
(https://github.com/bitcoin/bitcoin/pull/31840#issuecomment-2656921804)
> Our CMake toolchain for a Darwin cross build currently contains:
>
> ```shell
> set(CMAKE_AR "/usr/bin/llvm-ar")
> set(CMAKE_RANLIB "/usr/bin/llvm-ranlib")
> set(CMAKE_STRIP "/usr/bin/llvm-strip")
> set(CMAKE_OBJCOPY "arm64-apple-darwin-objcopy")
> set(CMAKE_OBJDUMP "/usr/bin/llvm-objdump")
> ```
>
> `objcopy` isn't currently used for the Darwin build (only for Linux and splitting the debug symbols), but we shouldn't be producing a toolchain file that refers to nonexistent tools.
...
(https://github.com/bitcoin/bitcoin/pull/31840#issuecomment-2656921804)
> Our CMake toolchain for a Darwin cross build currently contains:
>
> ```shell
> set(CMAKE_AR "/usr/bin/llvm-ar")
> set(CMAKE_RANLIB "/usr/bin/llvm-ranlib")
> set(CMAKE_STRIP "/usr/bin/llvm-strip")
> set(CMAKE_OBJCOPY "arm64-apple-darwin-objcopy")
> set(CMAKE_OBJDUMP "/usr/bin/llvm-objdump")
> ```
>
> `objcopy` isn't currently used for the Darwin build (only for Linux and splitting the debug symbols), but we shouldn't be producing a toolchain file that refers to nonexistent tools.
...
🤔 rkrux reviewed a pull request: "wallet: abandon orphan coinbase txs, and their descendants, during startup"
(https://github.com/bitcoin/bitcoin/pull/31794#pullrequestreview-2615195468)
Cursory review, will look again soon
(https://github.com/bitcoin/bitcoin/pull/31794#pullrequestreview-2615195468)
Cursory review, will look again soon
💬 rkrux commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1954619394)
`RecursiveUpdateTxState()` accepts a `const uint256& tx_hash` as the first argument but `tx.GetHash()` returns a `Txid&`, which is `Txid = transaction_identifier<false>`.
IIUC, this works because of this conversion function: https://github.com/bitcoin/bitcoin/blob/master/src/util/transaction_identifier.h#L67
A comment suggests for the new code to use `ToUint256` instead: https://github.com/bitcoin/bitcoin/blob/master/src/util/transaction_identifier.h#L62
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1954619394)
`RecursiveUpdateTxState()` accepts a `const uint256& tx_hash` as the first argument but `tx.GetHash()` returns a `Txid&`, which is `Txid = transaction_identifier<false>`.
IIUC, this works because of this conversion function: https://github.com/bitcoin/bitcoin/blob/master/src/util/transaction_identifier.h#L67
A comment suggests for the new code to use `ToUint256` instead: https://github.com/bitcoin/bitcoin/blob/master/src/util/transaction_identifier.h#L62