💬 hebasto commented on issue "32-bit Linux: build flags lost with depends & overriden CC(X)":
(https://github.com/bitcoin/bitcoin/issues/28096#issuecomment-1646515246)
This issue seems related to another one with overridden `CC` and `CXX` -- https://github.com/bitcoin/bitcoin/pull/23571.

💬 MarcoFalke commented on pull request "build: pass sanitize flags to instrument libsecp256k1 code":
(https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1646529778)
Maybe mark as draft for as long as CI is red?

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