💬 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?
(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
(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?
(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?
(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.
(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?
(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.
(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.
(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`?
(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
...
(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.
(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.
(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
...
(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
(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)
(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)
(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?
(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
...
(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.
(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..
(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
...
(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
...