💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1271271794)
That's a no-op assignment

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1271273469)
Thank you so much for noticing this. I don't know how I missed this.

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-1646540950)
Forced pushed fixing the Shadow variable issue.

💬 MarcoFalke commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#discussion_r1271273781)
Any reason for this?

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

* Got rid of the new `univalue_helpers` file again.
* Refactored the first commit to move the UniValue-specific code to `rpc/u
...

🤔 theStack reviewed a pull request: "test: Drop 22.x node from TxindexCompatibilityTest"
(https://github.com/bitcoin/bitcoin/pull/28070#pullrequestreview-1542075812)
Concept ACK

Small note: regarding using clean chain in feature_txindex_compatibility.py, is there any obvious advantage of re-generating blocks on every test-run rather than simply using the pre-generated one that's shared between tests? Probably both approaches are fine, Just wondering what's our general strategy here.

💬 MarcoFalke commented on pull request "test: Drop 22.x node from TxindexCompatibilityTest":
(https://github.com/bitcoin/bitcoin/pull/28070#issuecomment-1646547135)
Good question. IFF the cached test blockdir is stored in XOR format (https://github.com/bitcoin/bitcoin/pull/28052), it obviously can't be read by previous tests. So I can just drop the change here, and instead defer it to the later pull. Also, it may be good to include a compat test in the later pull?

👍 MarcoFalke approved a pull request: "doc: cleanup release process doc"
(https://github.com/bitcoin/bitcoin/pull/28003#pullrequestreview-1542076972)
lgtm

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271277231)
```suggestion
git commit -m "Add attestations by ${SIGNER} for ${VERSION} non-codesigned on [$(uname --machine)]"
```

Could include the architecture, since it is possible to use non-x86 machines to do the guix build?

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271276924)
any reason to drop this completely?

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271277474)
Can just drop this line (and above)? Alternatively, it should say to open a pull request after the push. Otherwise the push seems pointless.

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271277276)
Same?

💬 fanquake commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271285187)
It's no-longer relevant given the number no-longer exists.

💬 fanquake commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271285251)
Although I guess can keep the reminder to check for missing bips. Will re-add.

💬 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)