💬 MarcoFalke commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1627664179)
This doesn't compile. Ideally, you compile and test any changes locally before opening a pull request.

⚠️ MarcoFalke opened an 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)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

crash

### Expected behaviour

no crash

### Steps to reproduce

* Compile 79e8247ddb166f9b980f40249b7372a502402a4d
* Type `migratewallet`

### Relevant log output

```
wallet/scriptpubkeyman.cpp:1915: std::optional<MigrationData> wallet::LegacyScriptPubKeyMan::MigrateToDescriptor(): Assertion `IsMine(desc_spk) != ISMINE_NO' failed.
Aborted (core dumped)


### How did you obtain Bitcoi
...

🤔 theStack reviewed a pull request: "Add support for RFC8439 variant of ChaCha20"
(https://github.com/bitcoin/bitcoin/pull/27985#pullrequestreview-1520883441)
Nice and indeed much simpler now, the actual diff is smaller than I expected. Will re-review tomorrow.

Meanwhile, I thought it would be nice to have a test case triggering the 32-bit block counter overflow to check for >256 GiB output compatibility and wrote one (test data is created with PyCryptodome, see commit message), maybe it's worthwhile to include it here: https://github.com/theStack/bitcoin/commit/61b41ad16e35306485a1db508d624d3ddab0ca0a (the next commit https://github.com/theStack/b
...

💬 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-1627675932)
Now trying to start the program just fails on startup:

```
Error: Unrecognized descriptor found. Loading wallet ....../wallet.dat

The wallet might had been created on a newer version.
Please try running the latest software version.

👍 TheCharlatan approved a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1520884392)
Nice, ACK 4223d80bdae0d265e67280ea8853b184ff0ced13

Nit can be ignored.

💬 TheCharlatan commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1257458241)
Nit: `const CBlockIndex* block_to_test = start_block ? start_block : active_chain.Genesis();`

💬 theStack commented on pull request "test: Check expected_stderr after stop":
(https://github.com/bitcoin/bitcoin/pull/28028#discussion_r1257463070)
nit, logically equivalent, but likely less ambiguous for the reader's eyes:
```suggestion
assert (not expected_stderr) or wait_until_stopped # Must wait to check stderr
```

🤔 theStack reviewed a pull request: "test: Check expected_stderr after stop"
(https://github.com/bitcoin/bitcoin/pull/28028#pullrequestreview-1520888792)
Code looks good to me, will test locally tomorrow.

💬 sipa commented on pull request "Add support for RFC8439 variant of ChaCha20":
(https://github.com/bitcoin/bitcoin/pull/27985#issuecomment-1627726804)
@theStack Cherrypicked the first commit. I don't think the second one adds that much, as presumably there is no way the output can be correct if the internal state is wrong.

💬 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-1627746019)
To recover from the error, use the wallet backup. Can find it inside the directory of the wallet that you are trying to migrate e.g. `/wallets/wallet_name/wallet_name-<timestamp>.legacy.bak`.

The issue seems to be related to one of your imported scripts. Would be good to gather more information about the script type. So we can check under which circumstances the legacy spkm `IsMine` return false for a script contained by the wallet.

If you can, try applying this patch and re-run the migrat
...

💬 iemafzalhassan commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1627748360)
hey i am a newbie can anyone guide me what to do next to become a first contributor (this is my first project)

💬 jonatack commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1627763433)
> hey i am a newbie can anyone guide me what to do next to become a first contributor (this is my first project)

Hi @iemafzalhassan (and other future contributors), I suggest the articles section of https://jonatack.github.io.

💬 iemafzalhassan commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1627775797)
> > hey i am a newbie can anyone guide me what to do next to become a first contributor (this is my first project)
>
> Hi @iemafzalhassan (and other future contributors), I suggest the articles section of https://jonatack.github.io.

Thnkew so much 🤩🥳

💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1627778706)
> > hey i am a newbie can anyone guide me what to do next to become a first contributor (this is my first project)
>
> Hi @iemafzalhassan (and other future contributors), I suggest the articles section of https://jonatack.github.io.

Thanks a lot for this @jonatack!

💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1627779352)
> This doesn't compile. Ideally, you compile and test any changes locally before opening a pull request.

I see, I'll try to compile it on my end and will let you know if there's any issue. Also, can you please check the description for the new parameters added if they need any changes?

👍 theStack approved a pull request: "test: make assumeUTXO test capture the expected fatal error"
(https://github.com/bitcoin/bitcoin/pull/28050#pullrequestreview-1520939942)
Tested ACK 3e8bf2e10c26ef9c2f58307b523e4b674ac97a2c

💬 luke-jr commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1627785475)
AFAIK there is software which reads these files intentionally. IMO they can be expected to adapt, but having the option to disable it seems like a good idea in case they aren't ready right away.

💬 luke-jr commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1257521442)
```suggestion
{"longpollid", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Delay processing request until the result would vary significantly from the \"longpollid\" of a prior template."},
```

It doesn't necessarily imply the previous template is expired.

💬 luke-jr commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1257521831)
```suggestion
{"data", RPCResult::Type::STR_HEX, RPCArg::Optional::OMITTED, "Proposed block data to check, encoded in hexadecimal; valid only for mode=\"proposal\""},
```

✅ Brotcrunsher closed a pull request: "Supporting parameter "h" and "?" in -netinfo."
(https://github.com/bitcoin/bitcoin/pull/27830)