💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2743883179)
reACK 1601906941fa559ebbee7898453fa77f4606ad38
> Dropping the TxGraphImpl destructor body then causes segfaults (rather than a nice assertion failure).
Right, I got myself turned around on this thinking it was harder to achieve. I'd suggest adding coverage here or in a quick follow-up.
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2743883179)
reACK 1601906941fa559ebbee7898453fa77f4606ad38
> Dropping the TxGraphImpl destructor body then causes segfaults (rather than a nice assertion failure).
Right, I got myself turned around on this thinking it was harder to achieve. I'd suggest adding coverage here or in a quick follow-up.
💬 instagibbs commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2743888829)
> Would you rather the test failure be:
If you "walk" backwards to the functional test via the `comment` field it's ok, but I agree I would rather the tests cases be declared in the functional test rather than the dumped qa-assets version which lacks the context.
With regards to debugging, I found both this and https://github.com/bitcoin/bitcoin/pull/32114 similar in pain to review/fix, but I admit I've gotten used to gdb-attaching to bitcoind for functional tests regardless.
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2743888829)
> Would you rather the test failure be:
If you "walk" backwards to the functional test via the `comment` field it's ok, but I agree I would rather the tests cases be declared in the functional test rather than the dumped qa-assets version which lacks the context.
With regards to debugging, I found both this and https://github.com/bitcoin/bitcoin/pull/32114 similar in pain to review/fix, but I admit I've gotten used to gdb-attaching to bitcoind for functional tests regardless.
💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2743896375)
> Curious about policy on tooling since I'm currently working on UTXO analysis: it would be nice to refactor the (1) reading of the compressed UTXO format and (2) storing it in SQL format into separate components, so there's a reference for reading dumped UTXO sets people can readily use (e.g. `import CompressedUTXOReader from contrib/utxo-tools`). Or should code like that not be a part of Core?
If we do that, I think it would make most sense to move some routines or (new) classes to the func
...
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2743896375)
> Curious about policy on tooling since I'm currently working on UTXO analysis: it would be nice to refactor the (1) reading of the compressed UTXO format and (2) storing it in SQL format into separate components, so there's a reference for reading dumped UTXO sets people can readily use (e.g. `import CompressedUTXOReader from contrib/utxo-tools`). Or should code like that not be a part of Core?
If we do that, I think it would make most sense to move some routines or (new) classes to the func
...
📝 Sjors opened a pull request: "Rust tool to import bip39 mnemonic"
(https://github.com/bitcoin/bitcoin/pull/32115)
Since we're probably not going to support bip39 mnemonics in the wallet itself, but it's often requested feature, this PR provides a simple Rust utility to import one.
This puts the utility in `share` so it's included in releases. Alternatively we could make a separate repo akin to HWI.
Usage (first bip39 [test vector](https://github.com/trezor/python-mnemonic/blob/master/vectors.json)):
```sh
cd share/wallet
cargo run -- import bip39
Specify network: 'main' (default) or 'test' (te
...
(https://github.com/bitcoin/bitcoin/pull/32115)
Since we're probably not going to support bip39 mnemonics in the wallet itself, but it's often requested feature, this PR provides a simple Rust utility to import one.
This puts the utility in `share` so it's included in releases. Alternatively we could make a separate repo akin to HWI.
Usage (first bip39 [test vector](https://github.com/trezor/python-mnemonic/blob/master/vectors.json)):
```sh
cd share/wallet
cargo run -- import bip39
Specify network: 'main' (default) or 'test' (te
...
💬 Sjors commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-2743948319)
I wrote a little rust-bitcoin helper to generate a `importdescriptors` incantation for bip39 mnemonics, see #32115. If that makes it in, it could be expanded to handle codex32.
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-2743948319)
I wrote a little rust-bitcoin helper to generate a `importdescriptors` incantation for bip39 mnemonics, see #32115. If that makes it in, it could be expanded to handle codex32.
💬 Sjors commented on issue "support BIP39 mnemonic in descriptors":
(https://github.com/bitcoin/bitcoin/issues/19151#issuecomment-2743950601)
#32115 may be useful
(https://github.com/bitcoin/bitcoin/issues/19151#issuecomment-2743950601)
#32115 may be useful
📝 theStack opened a pull request: "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script"
(https://github.com/bitcoin/bitcoin/pull/32116)
The compact-size/varint/amount deserialization routines already exist in the functional test framework (with the advantage of having proper unit tests executed in CI), so we can just import them from there instead of leaving duplicated code in the repository.
(https://github.com/bitcoin/bitcoin/pull/32116)
The compact-size/varint/amount deserialization routines already exist in the functional test framework (with the advantage of having proper unit tests executed in CI), so we can just import them from there instead of leaving duplicated code in the repository.
🤔 darosior reviewed a pull request: "test: Add encodable PUSHDATA1 examples to feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32114#pullrequestreview-2706703935)
Nice. Concept ACK on having an example there.
(https://github.com/bitcoin/bitcoin/pull/32114#pullrequestreview-2706703935)
Nice. Concept ACK on having an example there.
💬 darosior commented on pull request "Rust tool to import bip39 mnemonic":
(https://github.com/bitcoin/bitcoin/pull/32115#issuecomment-2744013541)
> This puts the utility in `share` so it's included in releases
Does this mean the Guix build will now have to bootstrap a Rust compiler?
(https://github.com/bitcoin/bitcoin/pull/32115#issuecomment-2744013541)
> This puts the utility in `share` so it's included in releases
Does this mean the Guix build will now have to bootstrap a Rust compiler?
💬 Sjors commented on pull request "Rust tool to import bip39 mnemonic":
(https://github.com/bitcoin/bitcoin/pull/32115#issuecomment-2744136043)
@darosior no, the user has to `cargo run` it themselves. It's similar to the RPC auth script we ship, which also doesn't include Python itself: https://github.com/bitcoin/bitcoin/tree/master/share/rpcauth
(https://github.com/bitcoin/bitcoin/pull/32115#issuecomment-2744136043)
@darosior no, the user has to `cargo run` it themselves. It's similar to the RPC auth script we ship, which also doesn't include Python itself: https://github.com/bitcoin/bitcoin/tree/master/share/rpcauth
💬 murchandamus commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2744164157)
> In summery, two UTXOs are equivalent if both value and weight are the same, and if value is the same but weight is different, prefer the UTXO with lower weight since waste score is a derivative of weight.
Close, but there is a subtlety here. If we were sorting by effective_value and tie-breaking fee or weight, we would always be preferring lower weight, but instead, we tie-break on waste. That means that we will prefer lower weight if `fee_rate` is higher than `long_term_fee_rate`, but we wi
...
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2744164157)
> In summery, two UTXOs are equivalent if both value and weight are the same, and if value is the same but weight is different, prefer the UTXO with lower weight since waste score is a derivative of weight.
Close, but there is a subtlety here. If we were sorting by effective_value and tie-breaking fee or weight, we would always be preferring lower weight, but instead, we tie-break on waste. That means that we will prefer lower weight if `fee_rate` is higher than `long_term_fee_rate`, but we wi
...
💬 murchandamus commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#issuecomment-2744215852)
Oh I had missed this, and noticed it via the new fuzz target corpora. Great idea!
(https://github.com/bitcoin/bitcoin/pull/31870#issuecomment-2744215852)
Oh I had missed this, and noticed it via the new fuzz target corpora. Great idea!
💬 sipa commented on pull request "Rust tool to import bip39 mnemonic":
(https://github.com/bitcoin/bitcoin/pull/32115#discussion_r2008187157)
86, no?
(https://github.com/bitcoin/bitcoin/pull/32115#discussion_r2008187157)
86, no?
💬 Sjors commented on pull request "Rust tool to import bip39 mnemonic":
(https://github.com/bitcoin/bitcoin/pull/32115#discussion_r2008203657)
Indeed, fixed.
(https://github.com/bitcoin/bitcoin/pull/32115#discussion_r2008203657)
Indeed, fixed.
💬 murchandamus commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2744291162)
Could you please clarify the precision that you intend to use for sat/vB? I would recommend that we continue to use sat/kvB internally, maybe sat/kwu in the long-term, as we do only want to use integers but do want more precision than integer sat/vB. sat/kvB gives us three decimals of precision which seems sufficient.
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2744291162)
Could you please clarify the precision that you intend to use for sat/vB? I would recommend that we continue to use sat/kvB internally, maybe sat/kwu in the long-term, as we do only want to use integers but do want more precision than integer sat/vB. sat/kvB gives us three decimals of precision which seems sufficient.
👍 hodlinator approved a pull request: "build: Switch to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/30997#pullrequestreview-2705248957)
re-ACK 7b51bf1d4c1933677867f7364a2378e41bc23929
#### Changes (`git range-diff master 94967c353e 7b51bf1d4c`)
* [Disable Vulkan](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2740271781) in a different place to make sure support is properly disabled.
* Fixed invalid [cmake --parallel](https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2005045288) invocation.
* Less [hardcoded loglevel](https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2005002572).
* [Move tran
...
(https://github.com/bitcoin/bitcoin/pull/30997#pullrequestreview-2705248957)
re-ACK 7b51bf1d4c1933677867f7364a2378e41bc23929
#### Changes (`git range-diff master 94967c353e 7b51bf1d4c`)
* [Disable Vulkan](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2740271781) in a different place to make sure support is properly disabled.
* Fixed invalid [cmake --parallel](https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2005045288) invocation.
* Less [hardcoded loglevel](https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2005002572).
* [Move tran
...
💬 hodlinator commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2007165031)
Was thinking we have `dnf`-commands in build-unix.md. But I guess it's not so useful in build-unix.md since most distros have pre-built Qt lib packages, so one doesn't need ninja (and distros that build from source would have ninja as a dependency for Qt anyway).
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2007165031)
Was thinking we have `dnf`-commands in build-unix.md. But I guess it's not so useful in build-unix.md since most distros have pre-built Qt lib packages, so one doesn't need ninja (and distros that build from source would have ninja as a dependency for Qt anyway).
💬 sipa commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#issuecomment-2744295836)
@jurraca
The asmap binary format effectively just encodes a mapping from ip ranges to AS numbers by encoding them as a sequence of match instructions, conditional jumps, and return values.
A whole lot of ranges are unmapped, however, i.e., there just is no AS number for them. What the `--fill` option does is allow the encoder to map these unmapped ranges to anything, picking whatever is most efficient to encode. For example, if `123.45.0.0/24` and `123.45.2.0/23` map to AS12345, but `123.4
...
(https://github.com/bitcoin/bitcoin/pull/32110#issuecomment-2744295836)
@jurraca
The asmap binary format effectively just encodes a mapping from ip ranges to AS numbers by encoding them as a sequence of match instructions, conditional jumps, and return values.
A whole lot of ranges are unmapped, however, i.e., there just is no AS number for them. What the `--fill` option does is allow the encoder to map these unmapped ranges to anything, picking whatever is most efficient to encode. For example, if `123.45.0.0/24` and `123.45.2.0/23` map to AS12345, but `123.4
...
💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2008237122)
fixed
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2008237122)
fixed
💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2008237336)
yes added - nothing bad would happen if we didn't (because BLOCK_FAILED_CHILD would be recalculated at next startup) but I think it's definitely better to do it.
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2008237336)
yes added - nothing bad would happen if we didn't (because BLOCK_FAILED_CHILD would be recalculated at next startup) but I think it's definitely better to do it.