Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 sr-gi commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1480549987)
Why are we splitting the parsing of `NetPermissionFlags::Implicit` between here and `NetPermissions::Initialize`? I find this a bit weird
💬 sr-gi commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1481678841)
Any reason why this was moved from `Connman::CreateNodeFromAcceptedSocket` to here?
💬 sr-gi commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1480631259)
It may be worth documenting this in the method docs to prevent potential future footshots
💬 sr-gi commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1481727947)
nit: This comment is not relevant anymore. Either replace it with the one right before `self.noban_tx_relay = True` or remove it
💬 sr-gi commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1480633888)
Also, if kept, wouldn't it be better to set it to the default constant defined in `net_permission.h`?
💬 sr-gi commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1480624819)
What is the expected behavior if only the direction is specified? e.g. "in@1234"
💬 achow101 commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#issuecomment-1932412438)
> > To avoid this situation, we need to prevent the multiple batches from writing at the same time. Although SQLite does have facilities for doing that, they require the Write Ahead Log (WAL) mode, which is a feature that we explicitly chose to disable since it spread wallet data to multiple files. As such, we need to be handling the concurrency ourselves.
>
> This doesn't seem accurate to me. As long as all the batch objects are sharing a `sqlite3* m_db` pointer and using the same connection
...
💬 kevkevinpal commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1481757491)
should I update `disk` to `file` in this PR? I was trying to write the logs similar to the other ones in this file
💬 TheCharlatan commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#discussion_r1481702805)
Nit: No need for these formatting-only changes?
👍 TheCharlatan approved a pull request: "util: Faster std::byte (pre)vector (un)serialize"
(https://github.com/bitcoin/bitcoin/pull/29114#pullrequestreview-1868175505)
ACK fab41697a5448ef2861f65795bd63a4ccdda6a40

It's a nice cleanup, but I don't observe a big speedup on my machine after applying the diff and running the bench mentioned in the description on top of fab41697a5448ef2861f65795bd63a4ccdda6a40.
💬 sdaftuar commented on issue "Cluster mempool, CPFP carveout, and V3 transaction policy":
(https://github.com/bitcoin/bitcoin/issues/29319#issuecomment-1932465965)
Using a datalogging and replaying system that I have, I did some analysis of what the effect the idea of imbuing LN commitment transaction spends with v3 semantics would have had in 2023. My writeup of that is here: https://delvingbitcoin.org/t/analysis-of-attempting-to-imbue-ln-commitment-transaction-spends-with-v3-semantics/527
💬 hebasto commented on pull request "indicate explicit to the user that the wallet balances shown is watch only.":
(https://github.com/bitcoin-core/gui/pull/37#issuecomment-1932474790)
Closing due to lack of support from the author.
hebasto closed a pull request: "indicate explicit to the user that the wallet balances shown is watch only."
(https://github.com/bitcoin-core/gui/pull/37)
💬 furszy commented on pull request "init: settings, do not load auto-generated warning msg":
(https://github.com/bitcoin/bitcoin/pull/29301#discussion_r1481825039)
Sure, sounds good @ryanofsky. Thanks for coming back to this!
💬 hebasto commented on pull request "debugwindow: update session ID tooltip":
(https://github.com/bitcoin-core/gui/pull/788#issuecomment-1932536186)
> When you have a v2 connection, there is always a session ID.

What if the connection type is still `DETECTING`?

The code reference:https://github.com/bitcoin-core/gui/blob/6737331c4ccc6da578bb44c809cc4e18f017ba51/src/node/connection_types.h#L83-L88
👍 jarolrod approved a pull request: "release: Update translations for v27.0 soft translation string freeze"
(https://github.com/bitcoin/bitcoin/pull/29397#pullrequestreview-1868440624)
ACK 71927b24e5aceecd8a07cdaeb916898d45486bea

Pulled in update with the update-translations.py script and and updated the source files with `make -C src translate`, with a zero-diff
🚀 hebasto merged a pull request: "Change address / amount error background"
(https://github.com/bitcoin-core/gui/pull/553)
hebasto closed a pull request: "Bugfix: When restoring table columns, still set their minimum column width and stretch on last section"
(https://github.com/bitcoin-core/gui/pull/368)
💬 hebasto commented on pull request "Bugfix: When restoring table columns, still set their minimum column width and stretch on last section":
(https://github.com/bitcoin-core/gui/pull/368#issuecomment-1932610148)
Closing due to lack of activity.
💬 araujo88 commented on pull request "test: handle wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/pull/29395#issuecomment-1932611938)
> The issue is a race, so it is not deterministically reproducible. Usually, to deterministically reproduce it on all hardware, a sleep will have to be added in the right place in the source code or test code.
>
> A pull request will either have to explain where to put the sleep, or, if not possible, come up with a plausible explanation how the race could happen. Also, the pull request should explain if the bug is in the wallet code or in the test code, in which case the fix should be applied
...