Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 luke-jr opened a pull request: "Bugfix: net_processing: Restore "Already requested" error for FetchBlock"
(https://github.com/bitcoin/bitcoin/pull/28055)
Regression introduced by #27626

Adds new tests
📝 MarcoFalke converted_to_draft a pull request: "blockstorage: XOR blocksdir *.dat files"
(https://github.com/bitcoin/bitcoin/pull/28052)
Currently the *.dat files in the blocksdir store the data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan them and move them into quarantine, or delete them, or corrupt them. This may cause Bitcoin Core to fail a reorg, or fail to reply to P2P or RPC block requests.

Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a ran
...
📝 ItIsOHM opened a pull request: "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998"
(https://github.com/bitcoin/bitcoin/pull/28056)
This PR will add the optional parameters `longpollid` and `data` to `template_request` as they were missing when calling `help getblocktemplate` in RPCHelpMan.

I request the maintainers to review this and let me know about any mistakes in the descriptions of the parameters.

This PR refers to the issue #27998
💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1627627747)
Hello @stickies-v, sorry for the delay. I recently made a PR (https://github.com/bitcoin/bitcoin/pull/28056) and changed the description of the parameters, taking help from BIP 22 and BIP 23 as you suggested. But I am still unsure if it's a totally correct description or not, hence I have marked it as WIP.

I apologize if these changes aren't correct as I'm just a beginner here.
💬 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