Bitcoin Core Github
44 subscribers
119K links
Download Telegram
fanquake closed an issue: "26.0 RC Testing Guide Feedback"
(https://github.com/bitcoin/bitcoin/issues/28866)
💬 fanquake commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1838330241)
v26.0 has been tagged. Closing this issue.
fanquake closed an issue: "v26.0 Testing"
(https://github.com/bitcoin/bitcoin/issues/28718)
💬 fanquake commented on issue "v26.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1838331821)
v26.0 has been tagged. Closing this issue.
💬 maflcko commented on issue "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1838382894)
Removed from the 26.0 milestone. https://github.com/bitcoin/bitcoin/pull/28985 looks a bit large to backport, so I guess the fuzz target will be disabled on 26.x and the issue remains?
💬 hebasto commented on pull request "depends: Build the `native_capnp` and `capnp` packages with CMake":
(https://github.com/bitcoin/bitcoin/pull/28856#issuecomment-1838426302)
> Also seems fine if you don't want to cherrypick it here, since it's not directly related to this PR and might make it more confusing. I'm happy to open a new PR or review one if it doesn't get cherrypicked.

@ryanofsky

Please do. Thank you.
💬 fanquake commented on pull request "depends: Build the `native_capnp` and `capnp` packages with CMake":
(https://github.com/bitcoin/bitcoin/pull/28856#issuecomment-1838429144)
I'll just reopen my PR then. Not sure we need a third PR, for what is a really straightforward bugfix.
📝 fanquake reopened a pull request: "depends: fix libmultiprocess build on aarch64"
(https://github.com/bitcoin/bitcoin/pull/28846)
Two changes to improve the libmultiprocess build on aarch64.

First, is to always install libmultiprocess into `lib/`. On some systems (my Fedora aarch64 box), libmultiprocess was being installed into `lib64/`, and then configure would fail to pick it up, because we only add `lib/` to pkgconfig/ldflags out of depends. Rather than adding lib64 to those, I opted for installing libmultiprocess into lib, with every other dependency we build.

Second, is to build with position independant code. T
...
💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1413770472)
The idea is that `m_index` is always set to the next index in `m_offsets` that gets written to (so a value out of `[0, N)`) and we just assume that the initial N samples were 0, which is why the median is immediately computed out of N.

I could change it to always increase `m_index` instead and add new offsets by doing `m_offsets[m_index % N] = new_offset`, in which case your suggestion would allow us to compute the median more "accurately" while we haven't seen N samples yet. I'm not sure tho
...
💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1413773031)
Hm yea I guess... I could also just remove support for odd `N`s from this class.
💬 Sjors commented on pull request "Move log messages: tx enqueue to mempool, allocation to blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27277#issuecomment-1838518274)
Rebased after #28368.
💬 martinus commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1413803359)
The m_flags member here causes padding, when you move the member down above/below `flags` you should be able to save 8 bytes from the struct, then you can cache a bit more
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1413826104)
Sure. Will do if I have to retouch.
The number is to test the blocks download procedure and its moving window.
For instance, if the peer is at block 100, and it mines 300 blocks (288 + 12). The test verifies the node only download the blocks that are below the threshold from the limited peer (last 288 blocks minus the buffer). Then, the test connects a full node and it checks the node download the remaining historical blocks from it.
💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#discussion_r1413831223)
I can reproduce the issue. When the nodes catches up, for a small fraction of blocks it says "Saw new header via unsolicited block". The header refers to a block that was already connected, sometimes more than 25 blocks ago.
💬 andrewtoth commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1413832532)
Ah yes, it makes sense what `num_historical_blocks` purpose is. The nit is why the number 12 was chosen specifically. I suppose to just make it an even 300, and it can be arbitrary?
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1413837545)
> Ah yes, it makes sense what `num_historical_blocks` purpose is. The nit is why the number 12 was chosen specifically. I suppose to just make it an even 300, and it can be arbitrary?

Yep.
💬 fanquake commented on issue "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1838588474)
> will be disabled on 26.x and the issue remains?

Yea, #28985 just arrived way to late for 26.x, and it's still draft, tests failing, no review yet etc. I agree that as-is, it looks too big to backport, maybe it can be done in a way where there is 1 or 2 commits that can be cleanly cherry-picked to fix the issue in the release branch.
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1413843279)
@naumenkogs, do you agree on the naming as well? I'm happy to change it if we all agree.
Better to be sync to not circle much around the naming here.
💬 furszy commented on issue "Wallets should update key/descriptor birthdates when txs older than current birthdates are found":
(https://github.com/bitcoin/bitcoin/issues/28897#issuecomment-1838608012)
#28920 solves it. In case someone else wants to try it.