Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 Javadseyedi12 commented on pull request "scripted-diff: Modernize nLocalServices naming":
(https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2384288477)
Hello, good time, yes, what should I do?

در تاریخ دوشنبه ۳۰ سپتامبر ۲۰۲۴،‏ ۱۸:۱۵ Pieter Wuille <
***@***.***> نوشت:

> utACK 33381ea
> <https://github.com/bitcoin/bitcoin/commit/33381ea530ad79ac1e04c37f5707e93d3e0509ca>
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2384271156>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BLV7G4Q3UB43DPF6OJHTUM3ZZHEQ5AVCNFSM6AAAAABODX6MB6VHI2DSMVQWIX3LMV
...
💬 sipa commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2384291593)
I see both points of view here, and I have a hard time weighing them:
* There are certainly users for whom it is helpful that their Bitcoin P2P connections aren't as easily observable as V1 connection are, by their ISP or other entities, even if no strong hiding can be guaranteed.
* An option like this will likely result in a false sense of security for others, and I'm not comfortable with (eventually, if this were to be made the default) making it harder for older/other software to connect to
...
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2384295168)
Sorry for the repeated force pushes; I made a change in the wrong commit, and then made some minor improvements to the new fuzz too.

Absent non-nitty review that needs addressing, I don't intend to make change here anymore.
💬 iotamega commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2384325909)
> @jonatack While we should always be vigilant about the possibility of contributors with bad intentions, I think it is more productive to do so by judging contributions by their merit than by trying to infer people's intent.

I think intent here is pretty clear considering I am now working with CloudFlare to stop a DDOS attack on all of my sites.

I asked for this because of a friend in a country where they are having issues, very real issues and they did not want to doxx themselves but I a
...
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1781704314)
nit: same here, I'd remove "all"
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1781795191)
I've been thinking about leaking information about private broadcast via addrman:
1.) Here, we call `Good()`, possibly moving the peer from new to tried. Since tried table is smaller and sparsely filled especially for newer nodes, and addrs are picked with 50% probability from new/tried, this usually means that we are more likely to connect to them in the future - maybe via clearnet, maybe via future private connections.
2.) On disconnection, we call `Connected()`, changing the `nTime` of an a
...
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1781677003)
I would find the ThreadOpenConnection logic (which is already convoluted in master) easier to understand if the private broadcast related logic would be separated more clearly, e.g. this whole thing could be behind some bool `private_broadcast_enabled` so `m_private_broadcast.ShouldOpen` wouldn't be called in normal mode and one wouldn't need to scroll down and verify that this function does nothing then. Of course there is no functional difference, so it may be a matter of taste.
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1781868981)
Why store by `TxId` instead of `WtxId`? Would this add some attack surface, e.g. an attacker with many connections to peers receiving a private tx, and then sending each of their peers a modified version of the same tx with invalid witness data (but same TxId), in the hope of connecting to the originating node and clearing any further attempts to broadcast the transaction (node that `m_tx_for_private_broadcast.Remove()` also happens for invalid txs becaust it's done before validation)
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1781701897)
nit: all peers for which tx relay is enabled (so not blocks-only, feelers etc.).
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1781381298)
I found this help text hard to parse with all the "/" (had to read it multiple times to understand it). I'm not sure if the rpc help is the best place to give motivation for why features exist (instead of just describing them), but maybe this could be a bit more succinct?
💬 cobratbq commented on pull request "doc: external-signer, example 'createwallet' RPC call requires eight argument":
(https://github.com/bitcoin/bitcoin/pull/30354#issuecomment-2384385184)
In addition: documentation is completely lacking for flags `--stdin` and which stdin-commands to expect, including their format. Additionally, if we passing in to `stdin` directly, why wouldn't we pass the PSBT in binary form? Just have `signtx ` prefix then the binary data. (I guess this is too late now, but there is no reasoning or documentation about this anywhere in the spec.)
💬 jonatack commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2384388036)
> it holds significant promise for enhancing privacy and security, particularly in regions where internet freedoms are limited.

> in certain regions, Tor and I2P connections may be filtered out

Sincere question for my understanding, are there examples of regions where clearnet is unfiltered but tor/i2p/cjdns networks are blocked?

> I now see why they were so afraid, not just of the consequences of what the country would do but response from others here.

I think you're comparing very
...
⚠️ cobratbq opened an issue: "External signer: in case of error shows only "External signer failure""
(https://github.com/bitcoin/bitcoin/issues/31004)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Shows a dialog with title "External signer failure" and description "External signer failure".

### Expected behaviour

Report the error that is received as part of the execution result in the `error` field designated for such execution failures/errors.

### Steps to reproduce

Have an external signer perform a `signtx` (stdin) operation and report back a result that includes a body to the
...
⚠️ cobratbq opened an issue: "Docs: "External signer" documentation is very much outdated. (plz close if unwanted request)"
(https://github.com/bitcoin/bitcoin/issues/31005)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Specification documents some basic command-line options only.

### Expected behaviour

Including missing flags, at least `--account`, `--chain`, `--std`. And in case of `--stdin`, all further stdin-commands and corresponding results.

### Steps to reproduce

N/A

### Relevant log output

N/A

### How did you obtain Bitcoin Core

Other

### What version of Bitcoin Core are you using?

maste
...
💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count in GuessVerificationProgress":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1781940901)
Improving the error message is reasonably easy but I didn't see a way without expanding the chain interface. I have implemented what I think is a step in the right direction but not a full solution to every edge case in order to keep the scope of this PR reasonable. Looking for feedback if people still think this is right approach and if I should expand it further or if adding more is going too far.

I think the assumeutxo + pruning use-case is pretty hard to deal with accurately for these wal
...
💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count in GuessVerificationProgress":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2384474829)
Improved the error messages for `importdescriptors` and `rescanblockchain` to better match the reality (usage of pruning and assumeutxo).
💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2384561527)
rebased due to conflict in master
💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1781987204)
d'oh
💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2384565646)
reACK 2b21aebd9754c503bac12d1dbf566b3aa27451e8

via `git range-diff master 7051fcdda6fdc95677a34d5c14321d7580eceed8 2b21aebd9754c503bac12d1dbf566b3aa27451e8`
💬 petertodd commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2384612509)
A good question to ask here is if Bitcoin had shipped from day zero with V2-style encryption, would we ever "upgrade" the P2P protocol to turn it off? I believe the answer is clearly no: there's no downside to encrypting the P2P layer. So with regard to long-term upgrade plans, it's fine if in the long run everything eventually moves to V2, in the same way that we've done other backwards incompatible P2P upgrades in the past. This of course is just an obscure option, so there's no rush to contem
...