Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 yancyribbens commented on pull request "test: add assertions to SRD max weight test":
(https://github.com/bitcoin/bitcoin/pull/33058#issuecomment-3115059948)
Oh I see, SRD generates a different solution each time in the test-suit. That's annoying.
🤔 mzumsande reviewed a pull request: "Cache m_cached_finished_ibd where SetTip is called."
(https://github.com/bitcoin/bitcoin/pull/32885#pullrequestreview-3053316434)
> The function is only interesting when it can latch to the IBD finished state.
>
> That's only possible when all four conditions are met, which can only happen when the tip is updated.
>
> The final time based condition can only change when the tip changes as it gets further away with time, not closer.

I think that @luke-jr is right. If we reindex, we set `m_importing` to true in `ImportBlocks`, so any blocks we connect there can never result in getting out of IBD due to the `m_blockma
...
💬 mzumsande commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#discussion_r2229609944)
since the function is annotated with `EXCLUSIVE_LOCKS_REQUIRED(cs_main)` anyway, why not put it to the beginning of the function, as it is done in most other places?
💬 mzumsande commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#discussion_r2229577797)
First I thought this was not necessary because disconnecting a block shouldn't usually get you out of IBD, but it guess there are edge cases (starting up, with the old tip having a lower timestamp than it's parent block) where this could lead to get us out of IBD?!
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646189)
Done
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646353)
Made `void`
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646469)
Done
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646537)
Done
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646683)
I think leaving them here is fine.
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646764)
Done
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646853)
Done
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646929)
Done
💬 achow101 commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#issuecomment-3115094134)
ACK ec0b90e40e21ccb97c57373f7a7665ab9554acec

In a sample size of 1, this seems to provide a significant improvement in IBD time. It's surprising to me that it looks like this would have had benefit around the time that Taproot activated as previous measurements hadn't indicated that.
💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2229667232)
I've pulled in your suggestion here.
💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2229670245)
Not sure if it makes a measurable difference in performance, but I'm happy to use your version. Updated.
💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-3115124995)
Updated per review feedback (thanks!)
💬 achow101 commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3115210492)
Also seeing @mzumsande's observation if I add the addresses to loopback, but not if I add them to a different interface.

I think it's reasonable to filter out all loopback addresses, so the resolution is probably to just update the instructions to have the addresses bound to a non-loopback interface.
💬 monlovesmango commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#issuecomment-3115217031)
My feedback is mostly a lot of annoying nits mainly from leftover variables and comments still referring to old max announcements DoS scoring. I compiled them in my own [branch](https://github.com/monlovesmango/bitcoin/tree/pr32941-TxOrphanage-revamp-cleanups) so you can just cherry pick whatever you think is actually worth taking. Overview below.

- [doc: Fixes GetChildrenFromSamePeer comments](https://github.com/monlovesmango/bitcoin/commit/2a25c28aec3773bdf9a17148e45ce38ee95a7342) - Updates
...
💬 pinheadmz commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3115236229)
If @b-l-u-e is going to retouch to update the instructions in the test comment I'd also really like to see those instructions executed in a CI test as well.
💬 mzumsande commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3115248007)
For me it would also ok to leave the resolution of #31336 for a separate PR - but this PR shouldn't claim to solve the issue then.