Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 Gary19751957 commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#issuecomment-2446696590)
I want whoever you are off of my emails,and away from my crypto and
assets,I do not know you and have given no permissions for any one to
Access my account or any other information being my GitHub repositorys or
other wise.The FBI has been notified and given the names along with a copy
of this email to further this investigation.Gary Rollins

Gary Rollins

On Wed, Oct 30, 2024, 7:15 AM l0rinc ***@***.***> wrote:

> ***@***.**** commented on this pull request.
> -----------------------
...
💬 laanwj commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2446708418)
> I don't think just sending transactions over encrypted connections gives that much extra privacy.

i'm not advicating to send *just transactions*, v2 connections would have blocks+transactions, all of it. Not sending transactions over v1 makes sure that transactions aren't sent over the network in plaintext.

In the long term, if the idea is to phase out P2P v1, i think demotiing it to a blocksonly-protocol makes sense. It holds the network together against eclipse attacks and accidental
...
📝 allanlealluz opened a pull request: "aa"
(https://github.com/bitcoin/bitcoin/pull/31183)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
fanquake closed a pull request: "aa"
(https://github.com/bitcoin/bitcoin/pull/31183)
📝 fanquake locked a pull request: "aa"
(https://github.com/bitcoin/bitcoin/pull/31183)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1822430311)
In baac72581b1c99a41ab16a00a3f25fe938eb0610: "This prevents adversarial nodes from proving
our reconciliation set by spamming reconciliation requests": Couldn't we just check the last time the node did a reconciliation request and simply ignore the request?
💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1822431933)
Just talked to Bruno a bit more about this.

I think it's fair to compare this delayed set mechanism to what i have in the code there.
I like my solution more because:
- its synced across peers (handles better spying across multiple conns i think?)
- simpler code
- more flexibility (doesn't have to be same trickling as adding to sets or fanouting)

Interested in what other people think though.
💬 theStack commented on pull request "key: clear out secret data in `DecodeExtKey`":
(https://github.com/bitcoin/bitcoin/pull/31166#issuecomment-2446786655)
@davidgumberg: Good point, that makes a lot of sense. It seems there are more changes needed to make this work though, e.g. `DecodeBase58Check` currently only accepts a `std::vector<unsigned char>` with the default allocator, so one would need to template that function to accept `std::vector<unsigned char, T>` (if that works, didn't try). Also, assigning the `Base58Prefix` causes a similar type mismatch. Might be a potential follow-up if someone is motivated to look into that?
💬 laanwj commented on pull request "ci: Place datadirs for tests under tmpfs ":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2446807668)
Using tmpfs for temporary test directories can potentially speed up tests a lot, something to be wary for is that it significantly increases memory usage as well (no idea if this is a problem for the current CI).
💬 laanwj commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2446822174)
Concept ACK
👍 laanwj approved a pull request: "build: have "make test" depend on "make all""
(https://github.com/bitcoin/bitcoin/pull/31015#pullrequestreview-2404602445)
ACK 2957ca9611916efb570d157b9c7a0b188161660d
👋 maflcko's pull request is ready for review: "ci: Place datadirs for tests under tmpfs "
(https://github.com/bitcoin/bitcoin/pull/31182)
💬 maflcko commented on pull request "ci: Place datadirs for tests under tmpfs ":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2446967574)
> something to be wary for is that it significantly increases memory usage as well (no idea if this is a problem for the current CI).

I'd say the memory increase shouldn't matter, because on modern distros the default temp dir is already a tmpfs. So if memory was an issue, then running the tests normally (outside the CI) would already lead to issue reports. (I haven't seen any, but I may be missing some).

So I think it is fine to enable this by default without an off-switch for now. If the
...
💬 laanwj commented on pull request "ci: Place datadirs for tests under tmpfs ":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2446990530)
> I'd say the memory increase shouldn't matter, because on modern distros the default temp dir is already a tmpfs. So if memory was an issue, then running the tests normally (outside the CI) would already lead to issue reports. (I haven't seen any, but I may be missing some).

i don't think anyone runs the tests in low-memory/dis conditions 😄 i've had some issues on custom RISC-V hardware in the past, but nothing significant enough to create an issue about. At least the CI with start with a c
...
💬 naumenkogs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1822539819)
in a8d0976de7edf43d3cfea8dff3afc923580d8f84
⚠️ southunitedraza opened an issue: "Tokensg"
(https://github.com/bitcoin/bitcoin/issues/31185)
fanquake closed an issue: "Tokensg"
(https://github.com/bitcoin/bitcoin/issues/31185)
:lock: fanquake locked an issue: "Tokensg"
(https://github.com/bitcoin/bitcoin/issues/31185)
💬 andrewtoth commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2447160934)
> this approach is possible but is it maintainable?

Hmm, yeah this would end up with `reserve(<magic number>)` littered throughout the json serialization. Not ideal.

I took a quick look through most of the block parsing, and many of the reservations we would do would be <5, so unlikely to be a big benefit.

The `entry` in `TxToUniv` could have up to 12 kv-pairs stored, and in `blockheaderToJSON` it could have 14. These would also be reserving two vectors so might have some visible benefi
...
💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#issuecomment-2447182783)
Rebased #30933 on top of this PR and the mismatches between our custom `consteval` checking and tinyformat are gone as far as our current tests go - rebased commit: 32d4cf37d5056dc42bbf317b059e10910b984b0e