🤔 www222fff reviewed a pull request: "test: Ignore UTF-8 errors in assert_debug_log"
(https://github.com/bitcoin/bitcoin/pull/28035#pullrequestreview-1521251349)
Does this fix applied to both version for python2 and python3?
(https://github.com/bitcoin/bitcoin/pull/28035#pullrequestreview-1521251349)
Does this fix applied to both version for python2 and python3?
📝 hebasto opened a pull request: "refactor: Make more transaction size variables `int32_t`"
(https://github.com/bitcoin/bitcoin/pull/28059)
This PR is a continuation of https://github.com/bitcoin/bitcoin/pull/23962.
It gets rid of two static casts and is useful for https://github.com/bitcoin/bitcoin/pull/25972, see the failed ARM and multiprocess CI jobs.
(https://github.com/bitcoin/bitcoin/pull/28059)
This PR is a continuation of https://github.com/bitcoin/bitcoin/pull/23962.
It gets rid of two static casts and is useful for https://github.com/bitcoin/bitcoin/pull/25972, see the failed ARM and multiprocess CI jobs.
📝 hebasto converted_to_draft a pull request: "refactor: Make more transaction size variables `int32_t`"
(https://github.com/bitcoin/bitcoin/pull/28059)
This PR is a continuation of https://github.com/bitcoin/bitcoin/pull/23962.
It gets rid of two static casts and is useful for https://github.com/bitcoin/bitcoin/pull/25972, see the failed ARM and multiprocess CI jobs.
(https://github.com/bitcoin/bitcoin/pull/28059)
This PR is a continuation of https://github.com/bitcoin/bitcoin/pull/23962.
It gets rid of two static casts and is useful for https://github.com/bitcoin/bitcoin/pull/25972, see the failed ARM and multiprocess CI jobs.
💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1257771285)
ah, yes, you're right
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1257771285)
ah, yes, you're right
👋 hebasto's pull request is ready for review: "refactor: Make more transaction size variables `int32_t`"
(https://github.com/bitcoin/bitcoin/pull/28059)
(https://github.com/bitcoin/bitcoin/pull/28059)
💬 MarcoFalke commented on pull request "refactor: Make more transaction size variables `int32_t`":
(https://github.com/bitcoin/bitcoin/pull/28059#issuecomment-1628328861)
Not sure. The other types are 64 bit, so this will overflow eventually
(https://github.com/bitcoin/bitcoin/pull/28059#issuecomment-1628328861)
Not sure. The other types are 64 bit, so this will overflow eventually
💬 MarcoFalke commented on pull request "fuzz: Modify tx_pool_standard target to test package processing":
(https://github.com/bitcoin/bitcoin/pull/25778#issuecomment-1628332846)
Closing for now. Maybe this can be picked up as part of one of the package relay PRs.
(https://github.com/bitcoin/bitcoin/pull/25778#issuecomment-1628332846)
Closing for now. Maybe this can be picked up as part of one of the package relay PRs.
✅ MarcoFalke closed a pull request: "fuzz: Modify tx_pool_standard target to test package processing"
(https://github.com/bitcoin/bitcoin/pull/25778)
(https://github.com/bitcoin/bitcoin/pull/25778)
💬 MarcoFalke commented on pull request "test: make assumeUTXO test capture the expected fatal error":
(https://github.com/bitcoin/bitcoin/pull/28050#issuecomment-1628345098)
Also it may be good to name the commit that introduced the bug. Otherwise it will be harder for reviewers to see if there was a bug in that commit or if the change was intentional. Also, reviewers will be missing context.
(https://github.com/bitcoin/bitcoin/pull/28050#issuecomment-1628345098)
Also it may be good to name the commit that introduced the bug. Otherwise it will be harder for reviewers to see if there was a bug in that commit or if the change was intentional. Also, reviewers will be missing context.
💬 russeree commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1257799964)
8f2e548f6276386c27a60336cf12665d5e316fb7 solves this via the addition of a function to deterministically calculate the range set of encoded base58 prefixes that result from a version byte(s). The requirements are the length of bytes that are encoded and the desired prefix bytes as a vector.
As a side note thanks for suggesting this a review item, it was quite a fun problem to think about and implement a solution for.
https://github.com/bitcoin/bitcoin/blob/8f2e548f6276386c27a60336cf12665d
...
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1257799964)
8f2e548f6276386c27a60336cf12665d5e316fb7 solves this via the addition of a function to deterministically calculate the range set of encoded base58 prefixes that result from a version byte(s). The requirements are the length of bytes that are encoded and the desired prefix bytes as a vector.
As a side note thanks for suggesting this a review item, it was quite a fun problem to think about and implement a solution for.
https://github.com/bitcoin/bitcoin/blob/8f2e548f6276386c27a60336cf12665d
...
💬 russeree commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-1628375439)
This PR has had some substantial updates applied since it's first inception.
1. Deterministically calculated base58 prefixes
2. This PR now greatly improves the accuracy of error reporting as seen in the updates to 3d0a5c37e9ccedfa4ecfaa48eeeca1ada5b4eec1 where many valid bech32 tests are fed through the base58 section of the `DecodeDestination` function. This line gives a great example of how this PR addresses this from the current
https://github.com/bitcoin/bitcoin/blob/79e8247ddb166f9
...
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-1628375439)
This PR has had some substantial updates applied since it's first inception.
1. Deterministically calculated base58 prefixes
2. This PR now greatly improves the accuracy of error reporting as seen in the updates to 3d0a5c37e9ccedfa4ecfaa48eeeca1ada5b4eec1 where many valid bech32 tests are fed through the base58 section of the `DecodeDestination` function. This line gives a great example of how this PR addresses this from the current
https://github.com/bitcoin/bitcoin/blob/79e8247ddb166f9
...
💬 S3RK commented on pull request "test: Fixes and updates to wallet_backwards_compatibility.py for 25.0 and descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/28027#discussion_r1257851249)
v0.16 is different in many regards, does it make sense to have a separate test to check incompatibility?
(https://github.com/bitcoin/bitcoin/pull/28027#discussion_r1257851249)
v0.16 is different in many regards, does it make sense to have a separate test to check incompatibility?
💬 S3RK commented on pull request "test: Fixes and updates to wallet_backwards_compatibility.py for 25.0 and descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/28027#discussion_r1257854317)
pull up the comment explaining why?
(https://github.com/bitcoin/bitcoin/pull/28027#discussion_r1257854317)
pull up the comment explaining why?
💬 fanquake commented on pull request "test: Ignore UTF-8 errors in assert_debug_log":
(https://github.com/bitcoin/bitcoin/pull/28035#issuecomment-1628449877)
> Is this fix applied to both for python2 and python3?
We only support Python 3.
(https://github.com/bitcoin/bitcoin/pull/28035#issuecomment-1628449877)
> Is this fix applied to both for python2 and python3?
We only support Python 3.
💬 MarcoFalke 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-1628461546)
I tried on 25.0, where it will just fill the storage at full speed in a never ending loop (aborted after 30 minutes)

(https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1628461546)
I tried on 25.0, where it will just fill the storage at full speed in a never ending loop (aborted after 30 minutes)

💬 MarcoFalke 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-1628476666)
```
2023-07-10T08:19:38Z [qt-rpcconsole] [wallet/scriptpubkeyman.cpp:1917] [MigrateToDescriptor] MigrateToDescriptor: not mine script: addr(mqRzyzrW7DPGT1KZN2mLuyGPVUugpfEVPW)#4ndsysze
bitcoin-qt: wallet/scriptpubkeyman.cpp:1919: std::optional<MigrationData> wallet::LegacyScriptPubKeyMan::MigrateToDescriptor(): Assertion `IsMine(desc_spk) != ISMINE_NO' failed.
(https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1628476666)
```
2023-07-10T08:19:38Z [qt-rpcconsole] [wallet/scriptpubkeyman.cpp:1917] [MigrateToDescriptor] MigrateToDescriptor: not mine script: addr(mqRzyzrW7DPGT1KZN2mLuyGPVUugpfEVPW)#4ndsysze
bitcoin-qt: wallet/scriptpubkeyman.cpp:1919: std::optional<MigrationData> wallet::LegacyScriptPubKeyMan::MigrateToDescriptor(): Assertion `IsMine(desc_spk) != ISMINE_NO' failed.
💬 TheCharlatan commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1628492953)
Strong Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1628492953)
Strong Concept ACK.
💬 MarcoFalke commented on pull request "rpc: Drop migratewallet experimental warning":
(https://github.com/bitcoin/bitcoin/pull/28037#issuecomment-1628507151)
Would it make sense to document that this is expected to take a long time on spinning storage? For me a freshly created BDB wallet takes ~5 minutes to migrate. Another one I wasn't sure about, see https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1628461546.
Also, there seem to be outstanding bugs with the RPC, see same thread as above.
(https://github.com/bitcoin/bitcoin/pull/28037#issuecomment-1628507151)
Would it make sense to document that this is expected to take a long time on spinning storage? For me a freshly created BDB wallet takes ~5 minutes to migrate. Another one I wasn't sure about, see https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1628461546.
Also, there seem to be outstanding bugs with the RPC, see same thread as above.
💬 MarcoFalke commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1628658906)
Thanks for the feedback so far. I've kept the option to disable, but made it a simple bool instead of allowing the user to pick the XOR-key.
If someone is interested, a new commit can be added to upgrade the linearize scripts to read the XOR-key.
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1628658906)
Thanks for the feedback so far. I've kept the option to disable, but made it a simple bool instead of allowing the user to pick the XOR-key.
If someone is interested, a new commit can be added to upgrade the linearize scripts to read the XOR-key.
💬 peterwillcn commented on issue "mandatory-script-verify-flag-failed (Public key is neither compressed or uncompressed)":
(https://github.com/bitcoin/bitcoin/issues/21098#issuecomment-1628662930)
from_addr: 1HT7xU2Ngenf7D4yocz2SAcnNLW7rK8d4E same issues
(https://github.com/bitcoin/bitcoin/issues/21098#issuecomment-1628662930)
from_addr: 1HT7xU2Ngenf7D4yocz2SAcnNLW7rK8d4E same issues