Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🚀 fanquake merged a pull request: "test: make assumeUTXO test capture the expected fatal error"
(https://github.com/bitcoin/bitcoin/pull/28050)
💬 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-1629211279)
Hey @MarcoFalke should I revert the last commit? All checks except the lint test are passing.
👍 TheCharlatan approved a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1522386920)
re-ACK ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550
💬 sipa commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629226202)
Don't revert the commit; just remove the stray file and squash.
💬 vasild commented on issue "p2p sends local-scope addresses when IPv4 is behind NAT":
(https://github.com/bitcoin/bitcoin/issues/8616#issuecomment-1629232150)
To summarize, there are 3 issues here:

1. The problem from OP of sending local addresses to peers was nonexistent. That was never done.
2. Sending our addr in the `VERSION` message, fixed by https://github.com/bitcoin/bitcoin/pull/8740
3. Sending our addr via `ADDR` (or `ADDRv2`) message. I need to look at this more carefully, but a brief look tells me that maybe the problem exists. I mean sending our IPv4 address to a Tor peer or sending our Tor address to an IPv4 peer. At least `CNetAddr:
...
🚀 ryanofsky merged a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607)
💬 pinheadmz commented on pull request "wallet: Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1258472175)
> I'm not sure if or when legacy wallet support is expected to be phased out, but if isactive is only different from ismine in legacy wallets, is it worth adding a field (and all the new methods), versus just updating the ismine field documentation in the help for descriptor wallets? (When I'm double-checking an address with getaddressinfo before using it to receive, ismine is a field I'm already checking.)

@jonatack after diving back in to this issue I realized that `ismine != isactive` for
...
⚠️ ishaanam opened an issue: "Creating a Wallet Feature Guidelines Document"
(https://github.com/bitcoin/bitcoin/issues/28062)
It would be useful to the project if there was a document detailing what features are considered "out of scope" of the Bitcoin Core Wallet. This would make evaluating wallet PRs that add features easier for reviewers. It would also be useful to contributors so that they can get a better sense of which features are more likely to get merged. I'm curious if anybody else thinks that such a document would be useful, and if there are any guidelines that should be added. These guidelines would probabl
...
💬 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-1629250320)
> Don't revert the commit; just remove the stray file and squash.

I think it should be fine now, can you please check once again?
💬 sipa commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629252705)
You need to squash the commits. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 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-1629255377)
> You need to squash the commits. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

I think I did squash them, atleast that's what it shows in the commits tab.
💬 sipa commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629257234)
Apologies, you did indeed!
💬 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-1629260496)
> Apologies, you did indeed!

No worries, thanks for the guidance throughout this PR!
👍 theStack approved a pull request: "Add support for RFC8439 variant of ChaCha20"
(https://github.com/bitcoin/bitcoin/pull/27985#pullrequestreview-1522431786)
Code-review ACK 0bf87476f55dceb106563156c7c8d6bfb8162e29
💬 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-1629302650)
> Apologies, you did indeed! The commit title is "Deleted stray file `vcpkg`" now, though.

Should I amend this commit or leave it as it is?
👍 theStack approved a pull request: "test: Check expected_stderr after stop"
(https://github.com/bitcoin/bitcoin/pull/28028#pullrequestreview-1522532024)
ACK faf902858d38150caa8991b0ab9d7cfee2905684

Makes sense to do the stderr check at the very end after shutdown.
💬 sipa commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629352299)
Yes, amend the commit title to be accurate. You can rewrite the entire PR at any point.
💬 stickies-v commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1258032967)
nit
```suggestion
//! Result type for use with std::variant to indicate that an operation should be interrupted.
```
🤔 stickies-v reviewed a pull request: "kernel: Remove StartShutdown calls from validation code"
(https://github.com/bitcoin/bitcoin/pull/28048#pullrequestreview-1521747346)
Concept ACK
💬 stickies-v commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1258600239)
I don't see in which scenario `blockTip()` would return `kernel::Interrupted` here, but the docs seem to indicate that it could. I'd suggest to either update the docs to reflect that we never expect this to interrupt, or add a logging statement similar to what's done in `ThreadImport`?