<MacroFake>
Unless there are objections, I think it can be merged back for 24.0-final
AaronvanW has quit [Ping timeout: 260 seconds]
bitdex has quit [Quit: = ""]
<vasild>
LarryRuane: "Can `CChain::Next()` be called without holding `cs_main`?" -- In general, one has to check every variable that is read or written by CChain::Next() (directly or through other functions) -- how that variable is accessed in all other places.
<vasild>
The number of things to check could explode quite quickly. Proper thread annotations would help with that. Next() is quite small :)
<vasild>
It does access CChain::vChain in such a way that it expects it will not change between the reads. If it can be modified concurrently by other threads then that would be a problem.
<fanquake>
luke-jr: That pr contains changes that haven't even been merged into master, and would also be a last-minute change to translation strings, which as far as I'm aware, we avoid. So at this stage, no, it's not going to make it in.
<bitcoin-git>
[bitcoin] RandyMcMillan opened pull request #26505: src/bitcoin-cli.cpp: -getinfo help - grammar correction (master...1668522832/59e00c7e03/48174c0f28-getinfo) https://github.com/bitcoin/bitcoin/pull/26505
andrewtoth has quit [Remote host closed the connection]
andrewtoth has joined #bitcoin-core-dev
andrewtoth has quit [Remote host closed the connection]
andrewtoth has joined #bitcoin-core-dev
andrewtoth has quit [Remote host closed the connection]
andrewtoth has joined #bitcoin-core-dev
Guyver2 has joined #bitcoin-core-dev
halosghost has joined #bitcoin-core-dev
halosghost has quit [Read error: Connection reset by peer]
<jon_atack>
fanquake: The PR those backports depend on seems mergeable, along with the backports (thank you luke-jr for mentioning it).
<stevenroose>
Does Core store old blocks by height (i.e. sequentially)?
<jonatack>
Those backports may appear minor, but without them v24 will break user space. For example, the CashApp crypto team informed me that v24 will break their production systems if they upgrade without those patches.
<stickies-v>
jonatack: perhaps it would be useful if they could comment/create issue with a bit more detail on how this would break their stuff?
<stevenroose>
stickies-v: aha, thanks
<jonatack>
I've been proposing to fix these since August before v24 branch-off. If we go forward as-is, it seems to me the v24 release notes ought to warn about the changes (last I looked a few days ago, there was no mention of it).
<stevenroose>
stickies-v: and that holds for the entire chain, not just for the latest section of unprocessed blocks?
<stickies-v>
yeah, blocks just get serialized to disk when they're received and basically not touched afterwards, and receiving blocks is never guaranteed to be sequential
andrewtoth has quit [Remote host closed the connection]
<stickies-v>
jonatack: i know you've raised it a while ago but i think there is some unclarity on how these changes break user space? Hence having direct and specific user input could be helpful?
andrewtoth has joined #bitcoin-core-dev
<jonatack>
stickies-v: they use the endpoint for sanity checks in their CI and deployment. It's also common sense that a field always present since a number of years in the API shouldn't become sometimes present, sometimes absent, without a deeply compelling reason to do so -- in which case user space ought to be warned in that case.
<jonatack>
I've stated that case a number of times over months and need to sign off to present at a conf. Up to you all.
Guyver2 has left #bitcoin-core-dev [Closing Window]
<willcl_ark>
stevenroose: there is a script in contrib/linearize which can construct a linear block chain locally, if you require :)
jonatack has quit [Quit: WeeChat 3.7.1]
<stevenroose>
Things like Riccardo Casatta's block_iterator that iterates through the blockchain on disk would be significantly faster I presume if they would be sequential. As would a hypothetical streamblockchain RPC of some kind
salvatoshi has quit [Ping timeout: 260 seconds]
halosghost has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] brunoerg opened pull request #26507: test: remove unused vars in `feature_block` (master...2022-11-unused-feature-block-test) https://github.com/bitcoin/bitcoin/pull/26507
<bitcoin-git>
[gui] john-moffett opened pull request #680: Fixes MacOS 13 segfault by preventing certain notifications after main window is destroyed (master...202211MacSegfaultOnQuit) https://github.com/bitcoin-core/gui/pull/680
Talkless has joined #bitcoin-core-dev
sipsorcery has quit [Ping timeout: 260 seconds]
salvatoshi has quit [Remote host closed the connection]
salvatoshi has joined #bitcoin-core-dev
Zenton has quit [Read error: Connection reset by peer]
Zenton has joined #bitcoin-core-dev
bytes1440000 has joined #bitcoin-core-dev
<bytes1440000>
michaelfolkson: dont try to police what people write in any repo on github, time you spend on twitter or stackexchange, if spent on github and some testing maybe you could review some PR without bias
<bytes1440000>
or write some code at some point
salvatoshi has quit [Ping timeout: 268 seconds]
bytes1440000 has left #bitcoin-core-dev [#bitcoin-core-dev]
<lightlike>
jonatack: it's not clear to me what exactly breaks user space and should be mentioned in release notes. Do you mean the order change in getpeerinfo JSON output? Or that some fields can be reported differently very rarely during a specific moment during connection initialisation? Or something else?
Talkless has quit [Quit: Konversation terminated!]
<lightlike>
harding: yes, i saw that. I was asking about the "sometimes", to better understand how this affects user experience. Can this happen in a split second during connection initialization, and you'd need an extremely well-timed getpeerinfo call to evoke it? Can this happen in other circumstances?
<lightlike>
I just tried to add a sleep during various points in initialization with v24.0rc3 and tried to get a getpeerinfo reponse without minfeefilter present, but didn't succeed (but maybe I missed the right spot for the sleep).
gleb0712250 has joined #bitcoin-core-dev
<harding>
lightlike: I see. However, if we expect statestats to be available but we include an extra if statement just in case it isn't available, shouldn't we also expect downstream users to want to include their own if statements in their code case the minfeefilter RPC result isn't available?
NorrinRadd has quit [Ping timeout: 260 seconds]
<lightlike>
harding: Not sure if it makes sense to accomodate this particular situation if it's really just for an extremely short period during connection setup - maybe we could just not answer getpeerinfo until statestats is available? But to answer this (and to get an opinion whether this needs to be backported / mentioned in the release notes) the missing info for me is how exactly can it occur that statestats is not available.
<lightlike>
(I meant not include the particular peer in getpeerinfo, of course we should answer it)
vasild has quit [Remote host closed the connection]
yanmaani3 has quit [Remote host closed the connection]
shiza has quit [Remote host closed the connection]
generatecoll has quit [Quit: Client closed]
<lightlike>
harding, jonatack: I looked a bit more into this and wrote up my conclusions in #26457 - tldr is that I think the fields can only be missing if there is a race between a peer disconnecting and the getpeerinfo RPC.
<harding>
lightlike: whatever the outcome, thanks for the careful research!
hg has quit [Quit: WeeChat 3.7.1]
<luke-jr>
++lightlike
<luke-jr>
I think that means the fixes done are actually wrong, since it was assuming the opposite? (that it only occurs during connection setup)
NorrinRadd has quit [Ping timeout: 256 seconds]
sipsorcery has joined #bitcoin-core-dev
<lightlike>
luke-jr: I think they'd still fix the part where clients that would expect a field like "minfeefilter" to be always there won't crash - they'd just get incorrect data for these fields now. But the actual root cause that we'd sometimes report a mix of correct (but outdated) and wrong (default-initialized) data for an Peer that should be atomic is not fixed by this.