--- Log opened Wed Apr 23 00:00:24 2025
02:28 -!- pablomartin [~pablomart@89.35.30.155] has joined #bitcoin-core-pr-reviews
02:58 -!- S3RK_ [~S3RK@user/s3rk] has joined #bitcoin-core-pr-reviews
03:01 -!- S3RK [~S3RK@user/s3rk] has quit [Ping timeout: 260 seconds]
03:34 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has joined #bitcoin-core-pr-reviews
03:52 -!- fronti_ is now known as fronti
04:21 -!- PaperSword [~Thunderbi@securemail.qrsnap.io] has joined #bitcoin-core-pr-reviews
05:10 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
05:15 -!- pablomartin [~pablomart@89.35.30.155] has quit [Ping timeout: 245 seconds]
05:17 -!- pyth [~pyth@user/pyth] has quit [Ping timeout: 276 seconds]
05:17 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
05:22 -!- pyth [~pyth@user/pyth] has quit [Client Quit]
05:34 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
06:01 -!- PaperSword [~Thunderbi@securemail.qrsnap.io] has quit [Quit: PaperSword]
06:28 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Read error: Connection reset by peer]
06:29 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
06:32 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Remote host closed the connection]
06:41 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
06:47 -!- PaperSword [~Thunderbi@securemail.qrsnap.io] has joined #bitcoin-core-pr-reviews
06:51 -!- pablomartin [~pablomart@89.35.30.156] has joined #bitcoin-core-pr-reviews
07:05 -!- shesek2 [~shesek@user/shesek] has joined #bitcoin-core-pr-reviews
07:07 -!- shesek [~shesek@user/shesek] has quit [Ping timeout: 252 seconds]
07:07 -!- shesek2 is now known as shesek
08:20 -!- pablomartin [~pablomart@89.35.30.156] has quit [Ping timeout: 245 seconds]
08:47 -!- pablomartin [~pablomart@89.35.30.153] has joined #bitcoin-core-pr-reviews
09:27 -!- davesoma [~davesoma@2a02:1210:621d:e200:1bd:2160:23ef:c63b] has joined #bitcoin-core-pr-reviews
09:52 -!- pablomartin [~pablomart@89.35.30.153] has quit [Ping timeout: 252 seconds]
09:52 -!- amplia-rischia [~amplia-ri@212-8-250-219.hosted-by-worldstream.net] has joined #bitcoin-core-pr-reviews
09:57 -!- enochazariah [~enochazar@105.120.11.196] has joined #bitcoin-core-pr-reviews
09:57 -!- enochazariah [~enochazar@105.120.11.196] has quit [Client Quit]
09:57 -!- enochazariah [~enochazar@105.120.11.196] has joined #bitcoin-core-pr-reviews
09:58 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has joined #bitcoin-core-pr-reviews
09:59 -!- monlovesmango [~monlovesm@c-98-227-13-187.hsd1.il.comcast.net] has joined #bitcoin-core-pr-reviews
10:00 -!- enochazariah [~enochazar@105.120.11.196] has quit [Client Quit]
10:00 < glozow> hi
10:00 < monlovesmango> heyy
10:00 < davesoma> hi
10:00 < amplia-rischia> Hi there!
10:00 < abubakarsadiq> hi
10:00 < glozow> #startmeeting
10:00 < corebot> glozow: Meeting started at 2025-04-23T17:00+0000
10:00 < corebot> glozow: Current chairs: glozow
10:00 < corebot> glozow: Useful commands: #action #info #idea #link #topic #motion #vote #close #endmeeting
10:00 < corebot> glozow: See also: https://hcoop-meetbot.readthedocs.io/en/stable/
10:00 < corebot> glozow: Participants should now identify themselves with '#here' or with an alias like '#here FirstLast'
10:00 < sipa> hello!
10:01 -!- pseudoramdom [~pseudoram@user/pseudoramdom] has joined #bitcoin-core-pr-reviews
10:01 < glozow> Welcome to PR review club! Today is txgraph round 2, notes are available here: https://bitcoincore.reviews/31444
10:01 -!- Zenton [~Zenton@user/zenton] has quit [Read error: Connection reset by peer]
10:01 -!- enirox001 [~enirox001@105.120.11.196] has joined #bitcoin-core-pr-reviews
10:01 < glozow> Did anybody have a chance to review the PR or the notes?
10:02 < abubakarsadiq> I am in the process of reviewing the PR
10:02 < monlovesmango> I did review the best that I could
10:03 < glozow> Nice! What is your review approach/
10:03 < glozow> ?*
10:03 < monlovesmango> read PR review club notes, read through pr desc, read/skimmed through the commits, tried to answer pr review club questions
10:04 < abubakarsadiq> I like the approach, I am reviewing it commit by commit and running the fuzz test locally with modifications
10:04 < pseudoramdom> Hi! First time here. Did glance over the PR and the notes. Getting caught up on Cluster Mempool
10:04 < glozow> pseudoramdom: welcome!
10:04 < sipa> pseudoramdom: welcome to the (review) club!
10:04 < monlovesmango> oh yeah I also am runnign fuzz tests but it is taking forever and this is my first time so not sure what i'm doing
10:04 < glozow> Let's start with the questions
10:04 < glozow> Why are block building and eviction relevant to each other? Wouldn’t it be easier to evict transactions by the order they entered the mempool?
10:05 < glozow> Feel free to ask any of your own questions, whenever you like
10:05 < sipa> monlovesmango: just in case you're not aware - running fuzz tests generally runs indefinitely; it keeps trying to make randomized changes to the input, and seeing if those trigger code coverage change and (even better) assertion failures
10:06 < monlovesmango> they are both looking for the same data (ording of tx clusters by fee rate), just opposite goals. one wants the top fee rates, the other the lowest fee rate.
10:06 < abubakarsadiq>  glozow: you would want to evict the worst transaction in the mempool, i.e the one very unlikely to be mined soon.
10:06 < abubakarsadiq> 
As such when you use the order they enter mempool you will likely evict a transaction that might be mined in the next block.
10:06 < monlovesmango> it would be easier to evict by order they entered, but this can also evict your highest paying tx
10:07 < pseudoramdom> Does block building and eviction need to be opposites? or not necessary?
10:07 < sipa> monlovesmango: just a tiny nit, it's not sorting the *clusters* by feerate, but something else
10:07 < monlovesmango> sipa: haha ok thank you for letting me know! will have to look into how to run that properly
10:07 < monlovesmango> sipa: not clusters, chunks right?
10:07 < sipa> monlovesmango: bingo
10:07 < glozow> monlovesmango: abubakarsadiq: yes exactly
10:08 < sipa> pseudoramdom: today, they are very much not each other's opposite; the question is why we'd like them to be opposities
10:08 < glozow> to be fair, today, eviction is an approximation of the opposite of block building, just not an accurate one
10:09 < sipa> glozow: right, that's phrased more clearly
10:09 < glozow> True / false: if all clusters are singletons (have 1 transaction each), m_main_chunkindex would just be sorting the transactions by feerate
10:09 < monlovesmango> I want to say true
10:09 < abubakarsadiq> True :)
10:10 < pseudoramdom> I see. Yeah, it makes sense to have them accurately ordered. Block building can select the "best" end of the list. And eviction removes from the "worst" end
10:10 < glozow> Great :) can you explain why true?
10:10 < abubakarsadiq> If there is tie it it will compare the sequence of the clusters (individual tx)
10:11 < monlovesmango> if all clusters are singletons, then each cluster will have one chunk, and the linearization orders by chunk fee rate
10:11 < sipa> Bonus question: imagine two singleton clusters, with the same feerate, but they have different vsize. What can you say about their order?
10:11 < glozow> for the people following at home, we are looking at https://github.com/bitcoin-core-review-club/bitcoin/blob/27a0c93abb7e70b93214eb857e2046f848139e68/src/txgraph.cpp#L290-L306
10:12 < abubakarsadiq> sipa: thats a tie?
10:12 < sipa> abubakarsadiq: depends on your perspective :p
10:12 < glozow> monlovesmango: yes!
10:13 < abubakarsadiq> We have two comparators yes.
10:13 < pseudoramdom> m_sequence?
10:13 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has quit [Quit: Client closed]
10:15 < abubakarsadiq> sipa: I think the one with higher vsize will come first in the order since the sorting uses the > operator not `FeeRateCompare`?
10:15 < sipa> abubakarsadiq: no it uses FeeRateCompare, sorry - I thought it didn't, so this was a very trick question
10:16 < sipa> they'll be sorted by cluster creation order (m_sequence)
10:16 < Murch[m]> monlovesmango: You can just interrupt fuzz tests any time, or you can set a `max_total_time` or `runs` limit. You can ping me later if you want
10:16 < glozow> does `FeeRateCompare` return 0 for same feerate different vsize?
10:16 -!- Talkless [~Talkless@138.199.6.197] has joined #bitcoin-core-pr-reviews
10:16 -!- Guest80 [~Guest80@99.109.49.145] has joined #bitcoin-core-pr-reviews
10:16 < glozow> I suppose yes
10:17 < sipa> glozow: yes, FeeFrac::operator<=> (and derived operator<, ... etc) treat equal feerate objects as sorted by increasing size
10:17 < glozow> So it tie-breaks by sequence
10:17 < abubakarsadiq> ah, I've mentioned that as well above. and changed my mind :)
10:17 < sipa> but FeeRateCompare specifically just compares the feerate itself
10:17 < monlovesmango> Murch: ok sounds good thank you!
10:17 -!- davesoma [~davesoma@2a02:1210:621d:e200:1bd:2160:23ef:c63b] has quit [Quit: Client closed]
10:17 -!- davesoma [~davesoma@2a02:1210:621d:e200:1bd:2160:23ef:c63b] has joined #bitcoin-core-pr-reviews
10:18 < glozow> here: https://github.com/bitcoin-core-review-club/bitcoin/blob/27a0c93abb7e70b93214eb857e2046f848139e68/src/util/feefrac.h#L113
10:18 < glozow> Next question
10:18 < Murch[m]> (or rather, we can discuss here in this channel after the meeting, I think others might also chime in or want to read it)
10:18 < glozow> In English, using the approach in this PR, what is the algorithm for selecting transactions in order for block building? And for eviction?
10:19 < abubakarsadiq> @monlovesmango: I run the fuzz test to verify my understand that if I make modification to a commit I expect an assertion I added to be triggered which always do.
10:19 < glozow> You can discuss here, it's about the PR
10:19 < monlovesmango> glozow: I can't process too much at a time and would rather focus on questions :)
10:20 < monlovesmango> abubakarsa: that is a good idea!
10:21 < Murch[m]> glozow: Repeatedly pick from all clusters the chunk with the highest available chunk feerate until the block is full
10:21 < sipa> glozow: is your question more about what BlockBuildImpl does internally, or how you imagine high-level code would use BlockBuilder based on its interface?
10:22 < sipa> (because the actual block building code isn't inside this PR)
10:22 < glozow> let's start with: how would higher level code use BlockBuilder?
10:22 < abubakarsadiq> In English?
10:22 < glozow> (Oh oops, that's the next question)
10:22 < pseudoramdom> @glozow picking chunks in the order of highest to lowest feerate
10:22 < Murch[m]> abubakarsadiq: Hausa would be confusing to most of us :~
10:22 < sipa> Murch[m]: :D
10:22 < abubakarsadiq> :P
10:23 -!- dzxzg [~dzxzg@user/dzxzg] has joined #bitcoin-core-pr-reviews
10:24 < glozow> so let's start with: How would a client of BlockBuilder use it to build a block? When would GetCurrentChunk, Include, and Skip be called?
10:24 < monlovesmango> the algorithm for selecting transactions is to group related tx into clusters, and then linearize each cluster into chunks by their fee rate (this part i'm still fuzzy on), and then order all chunks by fee rate, and then pick chunks by decreasing fee rate (skipping chunks from clusters that have had a chunk skipped)
10:24 < glozow> monlovesmango: yes!
10:24 < monlovesmango> eviction is the same but starting from lowest fee rate
10:25 < abubakarsadiq> It's a linear algorithm that just get the current chunk recommended by block builder and include it when it satisfy a condition  or skip it when it didn't
10:25 < abubakarsadiq> Not sure why a chunk will be skipped is it because of blockmintxfee?
10:25 < pseudoramdom> GetCurrentChunk would give the "best" chunk available? 
10:25 < monlovesmango> i think if the cluster can't fit in a block?
10:26 < glozow> abubakarsadiq; can you think of a nother reason?
10:26 < glozow> monlovesmango; yes! but s/cluster/chunk
10:26 < glozow> What is the expected lifetime of BlockBuilder (is it similar to CTxMemPool’s or very different)?
10:26 < abubakarsadiq> thanks @monlovesmango
10:27 < monlovesmango> oh, but then why does it remember what to skip by cluster?
10:27 < glozow> monlovesmango: what happens if it doesn't?
10:27 < sipa> monlovesmango: once any chunk in a cluster has been skipped, nothing else from the cluster can't be included anymore, because the later transactions in the cluster may depend on transactions from the skipped chunk
10:27 < sipa> oh, sorry, spoiler
10:28 < abubakarsadiq> It is different, you should discard it immediately you are done building a block template.
10:28 < abubakarsadiq> TxGraph mutation methods can't be triggered  when we have a block builder instance; 
10:28 < monlovesmango> it will evaluate a chunk fromt he same cluster, which likely has missing dependencies! hah yeah what you said
10:28 < glozow> is it necessarily true that nothing else from the cluster can be included/
10:28 < glozow> ?* ugh my shift key
10:28 < sipa> glozow: you used it correctly for "English"
10:28 < monlovesmango> glozow: cool that makes a lot of sense thanks!
10:30 < abubakarsadiq> @sipa also if you skip a chunk in a cluster then that cluster linearization is incorrect yeah?
10:30 < sipa> abubakarsadiq: maybe
10:30 < monlovesmango> expected lifetime of BlockBuilder is short, as you can't make updates to txgraph if there is an observer right?
10:30 < glozow> If you skip something, you could still include its sibling, no?
10:31 < monlovesmango> should always be contained within CTxMemPool's lifetime
10:31 < sipa> abubakarsadiq: depends what you mean with correct; it won't be a linearization for the full cluster anymore as some transactions will clearly be missing, but it may still be a valid linearization for what remains of the cluster after removing the skipped chunk
10:31 < sipa> but BlockBuilderImpl (currently) doesn't try to reason about that because it's hard, and breaks the abstraction offered by linearizations/chunks
10:32 < glozow> monlovesmango: yes!
10:32 < sipa> so as soon as you skip anything of a cluster, it conservatively assumes nothing more from that cluster can be included
10:32 < abubakarsadiq> then if it is valid and topological even with the skipped chunk, arent miners losing on fees by skipping everything in the cluster?
10:32 < glozow> Why should `TxGraph` disallow changes while an observer exists?
10:32 < sipa> abubakarsadiq: yes, it is neccessarily an approximation, like block building always is
10:33 < sipa> abubakarsadiq: even the fact that we're only considering transactions in just a single order (the linearization) may result in small amounts of lost fees
10:33 < sipa> or the fact that it's an eager algorithm to begin with
10:34 < glozow> Can you create a BlockBuilder when staging exists? Can you build a block using the TxGraph’s state with its staged changes?
10:35 < monlovesmango> bc everytime TxGraph is updated it will need re-linearization, which you don't want to do while something is actively observing the ordering
10:35 < glozow> monlovesmango: exactly, it'll invalidate the chunk ordering that the observer is using
10:35 < pseudoramdom>  Why should `TxGraph` disallow changes while an observer exists? - the ordering mught change, possibility of missing a tx if the underlying graph changed? 
10:35 < abubakarsadiq> I think this case will likely happen towards the tail end of the block building process when we are close to the 4M `WU` limit. 
10:35 < abubakarsadiq> And also I think majority of tx are single clusters so it is fine?
10:36 < monlovesmango> yes I think you can't create a BlockBuilder when staging exists, but you can't build a block using staging
10:36 < sipa> monlovesmango: correct
10:36 < monlovesmango> you can* create a BlockBuiler
10:36 < sipa> oh, *can*, indeed
10:36 < monlovesmango> haha
10:37 < abubakarsadiq> @glozow: I think you can create a block builder while we have staging; but the recommended chunks will always be from main
10:37 < glozow> yep yep
10:37 < sipa> specifically, if you were making changes to main while a block builder exists, what would go wrong (ignoring the fact that it's not allowed, and might trigger Assume failes)
10:38 < sipa> glozow: tell me to shut up if my extra questions make us move too slowly
10:38 < monlovesmango> sipa: I would imagine things like double including a tx, or missing txs all together
10:38 < glozow> sipa: all good
10:39 < abubakarsadiq> @sipa; I think you will mess up with chunk order and invalidate the chunk index iterators?
10:39 < sipa> abubakarsadiq: yep, that's it
10:39 < sipa> the m_current_chunk iterator in BlockBuilderImpl may end up referring to a chunk index entry that no longer exists
10:39 < sipa> which would be undefined behavior in C++
10:39 < abubakarsadiq> yeah
10:40 < monlovesmango> ok interesting. is there no way that it would point to an inaccurate index, but one that does exist?
10:41 -!- dzxzg [~dzxzg@user/dzxzg] has quit [Quit: Client closed]
10:41 < sipa> monlovesmango: that is definitely possible, the point of undefined behavior is that it makes the behavior of the entire program undefined
10:41 < sipa> so it might do that
10:41 < glozow> Does `BlockBuilder` modify `TxGraph`?
10:41 < sipa> it might also result in sending all your BTC away
10:42 < abubakarsadiq> yep in constructor and destructor, while incrementing and decrementing block builder observer
10:42 < monlovesmango> yes, it modifies observer count
10:43 < sipa> might it make any other changes in the constructor?
10:43 < abubakarsadiq> peeping at the constructor.......
10:44 < monlovesmango> MakeAllAcceptable
10:44 < sipa> ✅
10:44 < glozow> tada https://github.com/bitcoin-core-review-club/bitcoin/blob/27a0c93abb7e70b93214eb857e2046f848139e68/src/txgraph.cpp#L2394
10:45 < monlovesmango> which looks like it does ApplyDependencies, which will mutate txgraph
10:45 < sipa> indeed
10:45 < abubakarsadiq> I second :)
10:45 < sipa> but it doesn't make any *observable* changes to the graph, as in, ApplyDependencies would have been called anyway, but possibly later
10:45 < glozow> In some ways, "no," because the observable contents of the graph don't change - BlockBuilder doesn't remove transactions for exampl
10:45 < abubakarsadiq> It might not though when their is nothing to apply
10:47 < glozow> We already answered Q9 before, so moving on to Q10
10:47 < glozow> looking at this commit https://github.com/bitcoin-core-review-club/bitcoin/commit/3429e9d79df1336cf1d0a61cb5f9bf028aa860b2
10:47 < glozow> This commit adds new fields in data structures that need to point to each other: Entry now contains an iterator to the transaction’s ChunkData in m_main_chunkindex, and ChunkData refrence Entrys by their position in m_entries. In your review, how did you check that these pointers are always kept up-to-date?
10:47 -!- dzxzg [~dzxzg@user/dzxzg] has joined #bitcoin-core-pr-reviews
10:49 < monlovesmango> wasn't able to finish trying to answer this question, but I would imagine that you want to check all the places where txgraph mutates (ApplyDependencies and Relinearize)
10:51 < glozow> Yeah, this is more of a "how do you review stuff?" question. I counted up the pointers and checked that there were assertions for them in `SanityCheck()`
10:51 < glozow> Conceptually, what are all the ways that an entry’s chunk index can change?
10:52 < pseudoramdom> when child tx is added/removed? 
10:52 < monlovesmango> glozow: can you explain more about SanityCheck()?
10:52 < monlovesmango> when a tx is added or removed from that entry's cluster?
10:53 < monlovesmango> i guess also if any tx is added/removed from mempool
10:53 < pseudoramdom> oops, there could be more scenarios - maybe when feerate changes by RBF
10:54 < glozow> monlovesmango: sure. generally I think that if pointer consistency is checked in `SanityCheck` and the fuzzer is good at finding possible (mis)uses of `TxGraph`, I can feel reasonably confident that `TxGraph` is updating those pointers correctly
10:54 < sipa> pseudoramdom: that's the same thing... chunk feerates are a function of the linearization of a cluster, so anything that changes the linearization can affect it... and that includes RBF, but that is effectively through the cluster changes by adding/removing transactions from it
10:55 < sipa> glozow: and as abubakarsadiq already mentioned, to get confidence that the fuzzer is indeed capable of finding such problems, it's often good to just try introducing a bug you expect SanityCheck (or other assertion) to find, and see if it actually does
10:55 < pseudoramdom> gotcha yeah, thanks for claryfying. (For a sec I was wondering my messages were not going thr') 
10:55 < glozow> monlovesmango: pseudoramdom: yeah, any of the mutators can change the chunk index
10:56 < sipa> also CommitStaging
10:56 < pseudoramdom> can clusters merge? 
10:56 < monlovesmango> glozow: interesting thank you!
10:56 < glozow> pseudoramdom: yep!
10:56 < sipa> pseudoramdom: yes, by adding a dependency between transactions that are in distinct clusters
10:56 < sipa> also, invoking the ~Ref destructor can change clusters
10:56 < sipa> because it causes the corresponding transaction to be removed
10:57 < glozow> In the ChunkOrder comparator, when cmp_feerate != 0, why can it be returned directly without comparing position within the cluster?
10:58 < monlovesmango> bc fee rate is the first priority when determining order?
10:58 < sipa> monlovesmango: but why is that ok?
10:59 < glozow> monlovesmango: but why doesn't that violate any dependencies?
11:00 < monlovesmango> bc we know they are from different clusters?
11:00 < monlovesmango> hmm no
11:00 < glozow> No, they can be within the same cluster
11:01 < glozow> But chunks within a cluster are already in feerate order :)
11:01 < glozow> Uh oh, we're already out of time!
11:01 < glozow> We got through a lot today, thanks for coming everybody
11:01 < glozow> #endmeeting
11:01 < corebot> glozow: Meeting ended at 2025-04-23T18:01+0000
11:01 < corebot> glozow: Raw log: https://achow101.com/ircmeetings/2025/bitcoin-core-pr-reviews.2025-04-23_17_00.log.json
11:01 < corebot> glozow: Formatted log: https://achow101.com/ircmeetings/2025/bitcoin-core-pr-reviews.2025-04-23_17_00.log.html
11:01 < corebot> glozow: Minutes: https://achow101.com/ircmeetings/2025/bitcoin-core-pr-reviews.2025-04-23_17_00.html
11:01 < monlovesmango> thanks for hosting glozow and sipa!!
11:02 < sipa> Thank you for hosting, glozow!
11:02 < abubakarsadiq> thanks for the notes @glozow and great PR by @sipa
11:02 < pseudoramdom> Thx! this was fun! 
11:02 < pseudoramdom> Thanks for hosting
11:02 < monlovesmango> I did have a couple extra questions that should be quick if sipa/glozow have time..
11:02 -!- enirox001 [~enirox001@105.120.11.196] has quit [Quit: Client closed]
11:02 < davesoma> thank you glozow!
11:02 -!- dzxzg [~dzxzg@user/dzxzg] has quit [Ping timeout: 240 seconds]
11:02 -!- Guest80 [~Guest80@99.109.49.145] has quit [Ping timeout: 240 seconds]
11:02 -!- pseudoramdom [~pseudoram@user/pseudoramdom] has quit [Remote host closed the connection]
11:03 < sipa> monlovesmango: happy to answer
11:03 -!- davesoma [~davesoma@2a02:1210:621d:e200:1bd:2160:23ef:c63b] has quit [Quit: Client closed]
11:03 < amplia-rischia> thank you guys, I am starting my journey and it's nice to see the support available to those who want to learn and contribute in the future
11:03 < monlovesmango> ok first one is can you point me to the line where GetMainStagingDiagrams is removing clusters that are same between main and staging?
11:04 < sipa> monlovesmango: i cannot
11:04 < sipa> because they're not removed
11:04 < monlovesmango> ahha ok
11:04 < monlovesmango> I guess is that planned? it seems to be in the pr desc and fuzz tests
11:05 < sipa> the implementation of main/staging in TxGraphImpl represents staging as a "diff" so to speak, containing only the clusters that are modified between them
11:05 < monlovesmango> ohhh ok
11:05 -!- amplia-rischia [~amplia-ri@212-8-250-219.hosted-by-worldstream.net] has quit [Quit: Client closed]
11:05 < sipa> so all GetMainStagingDiagrams has to do is return information from the clusters that are actually materialized in staging's representation, and all of main's corresponding clusters
11:06 < sipa> i think we touched on this in the previous txgraph review club
11:07 < monlovesmango> and second question is for the fuzz test, in txgraph.cpp why isn't the `else if` on line 669 not the last `else if`?
11:07 < sipa> can you point to the exact commit you're asking about?
11:07 < sipa> (because line numbers aren't stable, especially these)
11:08 < monlovesmango> sipa: ok that makes sense, yes I think that sounds familiar
11:09 < monlovesmango> the first commit
11:10 < monlovesmango> e1cb50a
11:10 < monlovesmango> sorry trying to get a link and struggling
11:11 < sipa> monlovesmango: which line here? https://github.com/bitcoin/bitcoin/blob/e1cb50a9574b70ae6509fc3578af3cbf886f2b63/src/test/fuzz/txgraph.cpp
11:12 < monlovesmango> ln 644
11:13 < monlovesmango> in general, there are many `else if` statements with `(command-- == 0)` condition, how does that work?
11:14 < sipa> each `else if` reduces command by 1; once command hits 0, that branch is used
11:14 < sipa> by putting conditionals before the (... && command-- == 0), it's possible to make each branch conditional
11:14 < sipa> DoWork can always be called
11:14 < sipa> GetMainStagingDiagrams cannot always be called
11:15 < sipa> but note that it's possible no condition at all is triggered, because command just starts off being too high
11:15 < sipa> in that case, the loop just repeats
11:15 < monlovesmango> but if you have multiple conditions which don't have and additional conditionals, how will the later branches ever hit?
11:16 < sipa> by command starting higher
11:16 < sipa> think of it this way: at every point, there are some subset of branches whose conditionals are met
11:17 < sipa> command starts at some number read off the fuzzer, and then iterates through all of those branches
11:17 < sipa> each of them decremening command
11:17 < sipa> once command==0 is reached, that one is used
11:17 < monlovesmango> but if you have 8 different branches that all have `else if (command-- == 0)` as the conditional, won't only the first one ever get hit? what is the difference between these 8?
11:18 < sipa> no
11:18 < sipa> say you have those 8, and all other branches have unmet conditionals
11:18 < sipa> if command starts at 0, the first branch will be used
11:18 < sipa> if command starts at 7, the last branch will be used
11:18 < sipa> if command starts at 8, the first branch will be used
11:19 < sipa> if command starts at 9, the second branch will be used
11:19 < sipa> etc
11:19 < monlovesmango> ok thank you! thats really weird and cool, will have to study it further
11:21 < monlovesmango> thats all i had thank you for your time :)
11:21 < sipa> yw!
11:22 < glozow> monlovesmango: thanks for all the hard work, make sure to leave a review on the PR! :)
11:23 < monlovesmango> glozow: ok I will!! thanks for the encouragement :)
11:40 -!- monlovesmango [~monlovesm@c-98-227-13-187.hsd1.il.comcast.net] has quit [Quit: leaving]
11:43 -!- dzxzg [~dzxzg@user/dzxzg] has joined #bitcoin-core-pr-reviews
11:52 -!- Guest80 [~Guest80@99.109.49.145] has joined #bitcoin-core-pr-reviews
11:58 -!- Guest80 [~Guest80@99.109.49.145] has quit [Quit: Client closed]
12:16 -!- Talkless [~Talkless@138.199.6.197] has quit [Quit: Konversation terminated!]
12:37 -!- dzxzg [~dzxzg@user/dzxzg] has quit [Quit: Client closed]
15:04 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has quit [Quit: Connection closed for inactivity]
15:10 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
15:16 -!- pyth [~pyth@user/pyth] has quit [Ping timeout: 276 seconds]
15:17 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
16:16 -!- ghost43_ [~ghost43@gateway/tor-sasl/ghost43] has joined #bitcoin-core-pr-reviews
16:17 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has quit [Ping timeout: 264 seconds]
16:27 -!- pyth [~pyth@user/pyth] has quit [Read error: Connection reset by peer]
16:29 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
17:15 -!- pyth [~pyth@user/pyth] has quit [Quit: Leaving]
18:07 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
19:02 -!- pyth [~pyth@user/pyth] has quit [Read error: Connection reset by peer]
19:03 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
19:11 -!- pyth [~pyth@user/pyth] has quit [Ping timeout: 252 seconds]
19:47 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Remote host closed the connection]
20:17 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
20:23 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 265 seconds]
22:12 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
22:17 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 245 seconds]
22:50 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
22:59 -!- pyth [~pyth@user/pyth] has quit [Read error: Connection reset by peer]
23:04 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
23:09 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 245 seconds]
23:15 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
23:46 -!- pyth [~pyth@user/pyth] has quit [Remote host closed the connection]
--- Log closed Thu Apr 24 00:00:25 2025