💬 brunoerg commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#discussion_r1271297592)
Isn't this required to use `ConnmanTestMsg`?

📝 furszy opened a pull request: "wallet: bugfix, disallow migration of invalid scripts"
(https://github.com/bitcoin/bitcoin/pull/28125)
Fixing #28057.

The legacy wallet allows to import any raw script, without checking if
it was valid or not. Appending it to the watch-only set.

This causes a crash in the migration process because we are only
expecting to find valid scripts inside the legacy spkm.

These stored scripts internally map to `ISMINE_NO` (same as if they
weren't stored at all..).

So we need to check for these special case, and take into account that
the legacy spkm could be storing invalid not watched sc
...

📝 furszy opened a pull request: "wallet legacy: bugfix, disallow importing invalid scripts via importaddress"
(https://github.com/bitcoin/bitcoin/pull/28126)
E.g. we're currently allowing to import scripts with several
sh levels.

These scripts are not being watched by the wallet;
`IsMine` returns `ISMINE_NO` for them (same as if they
weren't stored at all..).

So, there is no reason to accept them in the first
place.

Note:
To verify this, can run the test commit on top of master.
`wallet_basic.py --legacy-wallet` will fail without the
bugfix commit.

💬 furszy commented on issue "migratewallet crashes (wallet/scriptpubkeyman.cpp:1915: std::optional<MigrationData> wallet::LegacyScriptPubKeyMan::MigrateToDescriptor(): Assertion `IsMine(desc_spk) != ISMINE_NO' failed.)":
(https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1646592404)
I got creative and broke some stuff. @MarcoFalke, please try #28125.

📝 MarcoFalke opened a pull request: "Remove C-style const-violating cast, Use reinterpret_cast"
(https://github.com/bitcoin/bitcoin/pull/28127)
Using a C-style cast to convert pointer types to a byte-like pointer type has many issues:

* It may accidentally and silently throw away `const`.
* It forces reviewers to check that it doesn't accidentally throw away `const`.

For example, on current master a `const char*` is cast to `unsigned char*` (without `const`), see https://github.com/bitcoin/bitcoin/blob/d23fda05842ba4539b225bbab01b94df0060f697/src/span.h#L273 . This can lead to UB, and the only reason why it didn't lead to UB is b
...

⚠️ Elvis-Ashley opened an issue: "Make your trading journey smooth and easy."
(https://github.com/bitcoin/bitcoin/issues/28128)
Trading involves risk exercise caution while placing orders and solely consider market recommendations. Don't hesitate to seek for proper guidance from a professional. 𝐅𝐑𝐀𝐍𝐊𝐉𝐀𝐒𝐎𝐍𝐅𝐗_𝐎𝐅𝐅𝐈𝐂𝐈𝐀𝐋_𝐓𝐑𝐀𝐃𝐄𝐑 𝐎𝐍 𝐈𝐍𝐒𝐓𝐀𝐆𝐑𝐀𝐌 is always available for you. Get to learn proven strategies, risk management techniques, key insights, how to navigate the cryptocurrency markets and more from Frank Jason's trading platform

✅ fanquake closed an issue: "Make your trading journey smooth and easy."
(https://github.com/bitcoin/bitcoin/issues/28128)

:lock: fanquake locked an issue: "Make your trading journey smooth and easy."
(https://github.com/bitcoin/bitcoin/issues/28128)

💬 MarcoFalke commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#discussion_r1271303125)
Yeah, I am asking for the compiler warning when it is removed, or the method that requires it. Am I missing something?

💬 theStack commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1271314036)
Didn't verify my theory in practice yet, but according to my calculations having a HRP of size 4 would exceed the current 117 character limit (see https://github.com/bitcoin/bips/pull/1458/files#r1271312933). I assume the regtest-HRP should just also be set to "tsp" here (as it's currently stated in the BIP)? Otherweise the limit has to be raised by one.

Fun fact: in #27827 the "sprt" HRP works, but this is because the pubkeys are encoded as x-only (see commit https://github.com/bitcoin/bitco
...

💬 martinus commented on pull request "refactor: extract CCheckQueue's data handling into a separate container "Bag"":
(https://github.com/bitcoin/bitcoin/pull/27331#issuecomment-1646624160)
Thanks @jonatack , I've rebased 6b0537c -> 6a9c6ea. Fixed Makefile & include conflict, and added a `reserve()` in a unit test to fix a clang-tidy warning.

💬 brunoerg commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#discussion_r1271318560)
In fact, since we're using only `AddTestNode` I think that it's not required. Going to remove it..

💬 manfreddd commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646636639)
I was not sure how best to get the gettxoutsetinfo muhash output. So, I ran Bitcoin Core three different ways:

1. Run from the command line with coinstatsindex option (Bitcoincore.org doc recommends it).
[debug CLI coinstatsindex.log](https://github.com/bitcoin/bitcoin/files/12136921/debug.CLI.coinstatsindex.log)

3. Run from the command line with no options.
[debug CLI.log](https://github.com/bitcoin/bitcoin/files/12136923/debug.CLI.log)

4. Run from the graphical user interface and ge
...

💬 MarcoFalke commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646638775)
> Run from the graphical user interface and `gettxoutsetinfo muhash` from command window (crashed within 10 minutes).


It shouldn't crash, so there may be data corruption that leads leveldb into a crash.

I am not sure how to debug crashes on Windows, so someone else will need to help you there. (Maybe gdb exists?)


Otherwise, you can discard the leveldb chainstate by doing a full `-reindex-chainstate` (takes a long time).

💬 manfreddd commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646647499)
@MarcoFalke thank you for the support effort. Do I lose the Bitcoin data base loaded on my machine by discarding the leveldb chainstate?

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1646648548)
Updated 67e172a7c0d82271a13f77f76ce72799faddd05c -> b89567f51ade926af8c918e4787046b7ccec8eb0 ([kernelRmUnivalue_3](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_3) -> [kernelRmUnivalue_4](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmUnivalue_3..kernelRmUnivalue_4))

* Fixed fuzz test by catching the UniValue exception.

🤔 hebasto reviewed a pull request: "kernel: Remove UniValue from kernel library"
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1542134637)
Approach ACK b89567f51ade926af8c918e4787046b7ccec8eb0. Nice!

@TheCharlatan

Do I understand you correctly that your intention is to make the `ParseSighash` quite generic?

💬 hebasto commented on pull request "Remove C-style const-violating cast, Use reinterpret_cast":
(https://github.com/bitcoin/bitcoin/pull/28127#issuecomment-1646655381)
> Using a C-style cast to convert pointer types to a byte-like pointer type has many issues

Can `clang-tidy` or other tools be helpful in catching such cases in the future?

💬 emc99 commented on pull request "Remove C-style const-violating cast, Use reinterpret_cast":
(https://github.com/bitcoin/bitcoin/pull/28127#issuecomment-1646657942)
> Using a C-style cast to convert pointer types to a byte-like pointer type has many issues:
>
> * It may accidentally and silently throw away `const`.
> * It forces reviewers to check that it doesn't accidentally throw away `const`.
>
> For example, on current master a `const char*` is cast to `unsigned char*` (without `const`), see
>
> https://github.com/bitcoin/bitcoin/blob/d23fda05842ba4539b225bbab01b94df0060f697/src/span.h#L273
>
> . This can lead to UB, and the only reason why
...

💬 sipa commented on pull request "Remove C-style const-violating cast, Use reinterpret_cast":
(https://github.com/bitcoin/bitcoin/pull/28127#issuecomment-1646658996)
UB = Undefined behavior