--- Log opened Wed Oct 08 00:00:04 2025
00:15 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
00:20 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 255 seconds]
00:33 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
01:24 -!- alvroble [~alvroble@user/alvroble] has joined #bitcoin-core-pr-reviews
01:25 -!- alvroble [~alvroble@user/alvroble] has quit [Client Quit]
01:35 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 256 seconds]
01:53 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
01:58 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 256 seconds]
02:11 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
03:15 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 240 seconds]
03:29 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
04:23 -!- jerryf [~jerryf@user/jerryf] has quit [Remote host closed the connection]
04:23 -!- jerryf [~jerryf@user/jerryf] has joined #bitcoin-core-pr-reviews
04:31 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 256 seconds]
04:54 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
04:59 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 264 seconds]
05:28 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
05:32 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 240 seconds]
06:07 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
06:12 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 264 seconds]
06:17 -!- jerryf [~jerryf@user/jerryf] has quit [Remote host closed the connection]
06:18 -!- jerryf [~jerryf@user/jerryf] has joined #bitcoin-core-pr-reviews
06:24 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
06:30 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Read error: Connection reset by peer]
06:31 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
07:25 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has joined #bitcoin-core-pr-reviews
07:34 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has quit [Ping timeout: 250 seconds]
07:38 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has joined #bitcoin-core-pr-reviews
07:47 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has quit [Quit: Client closed]
08:22 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has joined #bitcoin-core-pr-reviews
08:41 -!- jerryf_ [~jerryf@user/jerryf] has joined #bitcoin-core-pr-reviews
08:42 -!- jerryf [~jerryf@user/jerryf] has quit [Remote host closed the connection]
08:45 -!- ___nick___ [~quassel@82-132-213-36.dab.02.net] has joined #bitcoin-core-pr-reviews
08:47 -!- alvroble [~alvroble@user/alvroble] has joined #bitcoin-core-pr-reviews
09:52 -!- dzxzg [~dzxzg@user/dzxzg] has joined #bitcoin-core-pr-reviews
09:59 -!- stringintech [~stringint@user/stringintech] has joined #bitcoin-core-pr-reviews
10:00 < marcofleon> #startmeeting
10:00 < corebot`> marcofleon: Meeting started at 2025-10-08T17:00+0000
10:00 < corebot`> marcofleon: Current chairs: marcofleon
10:00 < corebot`> marcofleon: Useful commands: #action #info #idea #link #topic #motion #vote #close #endmeeting
10:00 < corebot`> marcofleon: See also: https://hcoop-meetbot.readthedocs.io/en/stable/
10:00 < corebot`> marcofleon: Participants should now identify themselves with '#here' or with an alias like '#here FirstLast'
10:00 -!- monlovesmango [~monlovesm@37.19.200.166] has joined #bitcoin-core-pr-reviews
10:00 < marcofleon> hey everyone!
10:01 < stickies-v> hi!
10:01 < stringintech> Hi!
10:01 < kevkevin> hi
10:01 < dzxzg> hi
10:01 < monlovesmango> hello!
10:01 < danielabrozzoni> hi :)
10:01 < marcofleon> we're reviewing #33300 today
10:01 < corebot`> https://github.com/bitcoin/bitcoin/issues/33300 | fuzz: compact block harness by Crypt-iQ · Pull Request #33300 · bitcoin/bitcoin · GitHub
10:02 -!- yuvicc [~yuvicc@user/yuvicc] has joined #bitcoin-core-pr-reviews
10:02 < marcofleon> feel free to jump in whenever as we go through the questions, or circle back to previous ones, etc
10:02 < marcofleon> okay first one
10:02 < yuvicc> hi
10:02 < marcofleon> Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK? What was your review approach? Were you able to get the fuzz test running?
10:02 < stickies-v> if anyone's new the review club today, feel free to say hi - even if you're just lurking!
10:03 < marcofleon> oh yeah^
10:03 < kevkevin> I did not read the PR but I am lurking today!
10:03 < marcofleon> lurkers are welcome
10:03 < yuvicc> same here
10:03 < stringintech> while trying to get the fuzz test running on mac, i ran into a couple of compile-time and runtime issues. small write-up for the compile fix here: https://stringintech.github.io/blog/p/fuzzing-bitcoin-core-using-afl-on-apple-silicon . i will try to enhance/extend it once i wrap my head around the runtime issues (reported one on the PR page).
10:03 < stringintech> and a very light review. mostly to see how random input bytes turn into meaningful data for this fuzz test.
10:04 < danielabrozzoni> Concept ACK, I am still finishing my review :) I looked at commits one by one, trying to understand why each change was needed. I was able to run the fuzz tests.
10:04 < dzxzg> Concept ACK, I lightly read `strc/test/fuzz/cmpctblock.cpp` and got the fuzz test running
10:04 < monlovesmango> i was able to review a bit run the fuzz test. generally ack but have sociwmae fuzz related conceptual questions.
10:04 -!- Talkless [~Talkless@138.199.6.197] has joined #bitcoin-core-pr-reviews
10:04 < monlovesmango> a couple*
10:05 < stickies-v> also get in my usual cycle of not fuzzing for long enough and then no longer being able to get it to compile on macos arm
10:05 < marcofleon> stringintech: nice, yeah I saw your comments. I still need to look into it more to understand but I do wonder why that wasn't showing up on linux
10:05 < marcofleon> cool so we got people fuzzing!
10:06 < stringintech> marcofleon: yeah, that was also confusing for me...
10:06 < marcofleon> Did anybody get to trying the exercise?
10:06 < marcofleon> I just think it's fun to see a crash, that's why I ask
10:06 < stickies-v> concept ACK, spent too much time thinking about the (not very elegant, but maybe necessary) new startup option to copy datadirs
10:07 < marcofleon> monlovesmango: feel free to ask fuzz related questions whenever, I can try my best to answer
10:07 < marcofleon> stickies-v:  yeah agreed that part was a bit confusing to me too. We'll get into it
10:07 < marcofleon> okay second q
10:08 < marcofleon> Where in the codebase are the main compact block helpers and processing logic? Name some of the relevant classes and functions. (Hint: nothing like a little search for NetMsgType::CMPCTBLOCK to get you started)
10:08 < danielabrozzoni> I tried to see it crash, but couldn't reproduce! It might be that I haven't run the fuzzer for long enough, and my exec/sec are quite low
10:08 < monlovesmango> well I don't want to derail anything but usually with fuzz tests you see asserts everywhere, but here I didn't see any. so my question is where/how are we asserting that results are expected?
10:09 < stringintech> marcofleon: i could not unfortunately. blocked by the runtime issues. but had a question on this... how much the initial input we start with matters?
10:09 < marcofleon> danielabrozzoni: I had that thought after the notes went up unfortunately... "oh maybe it needs to run too long to actually reproduce"
10:10 < marcofleon> Okay in terms of the asserts, I think this harness is meant to be catching assumes/asserts we have in production code
10:10 < marcofleon> just generally testing compact block logic. But that's good that you're thinking about that monlovesmango because yeah in general you want good oracles for fuzz tests
10:11 < stickies-v> marcofleon: `PeerManagerImpl::NewPoWValidBlock` creates a `NetMsgType::CMPCTBLOCK` message and through `m_connman.ForEachNode` sends it to our high-bandwidth peers
10:11 < dzxzg> The `PartiallyDownloadedBlock` class for all of the compact block construction object stuff, and a lot of the logic is inside of the `ProcessMessage` NetMsgType::CMPCTBLOCK case
10:11 < marcofleon> and then stickies, initial input doesn't matter, the fuzzer will find its way
10:12 < monlovesmango> it seemed like class CBlockHeaderAndShortTxIDs in blockencodings.h was putting together the compact block message. is that the question?
10:12 < danielabrozzoni> marcofleon: Ah, got it! I'll try to run it for longer, just for fun. Maybe I could also modify the harness so that it will get to the crash quicker, as an exercise 
10:12 < marcofleon> unless you need very specific strings for serialization or network things etc... so I guess it can matter. but not too often afaik
10:12 < monlovesmango> marcofleon: aahhh that makes a lot of sense thanks!
10:12 < stringintech> thanks!
10:13 < stringintech> for question 2: this might be way too simplified but seems we can divide main parts into 3 categories:
10:13 < stringintech> 1) SEND a peer a cmpct block in HIGH bandwidth mode: in `PeerManagerImpl::NewPoWValidBlock()` callback (triggered by `ChainstateManager::AcceptBlock()`) and in `PeerManagerImpl::SendMessages()` (in case not already sent the cmpct block in `PeerManagerImpl::NewPoWValidBlock()` to this peer)
10:13 < stringintech> 2) SEND a peer a cmpct block in LOW bandwidth mode: process peer cmpct block request in `PeerManagerImpl::ProcessGetBlockData()` and potentially read the block from disk and construct the cmpct block - if not already done - and send it; constructing the cmpct block happens through `CBlockHeaderAndShortTxIDs` constructor in blockencodings.cpp
10:13 < stringintech> 3) RECEIVE a cmpct block from a peer: in `PeerManagerImpl::ProcessMessage()` processing `NetMsgType::CMPCTBLOCK` msg type, we process the cmpct block sent to us and try to construct the full block using txns in our mempool (using `PartiallyDownloadedBlock::InitData()` in blockencodings.cpp) and potentially send a `NetMsgType::GETBLOCKTXN` msg to
10:13 < stringintech> the same peer for the txns we did not have where we can later process the response in `PeerManagerImpl::ProcessCompactBlockTxns()` and finally call `PartiallyDownloadedBlock::FillBlock()` to see if we have the full block now.
10:13 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has quit [Quit: Connection closed for inactivity]
10:13 < marcofleon> Yeah that question was just to get people going through the compact block code
10:13 < stringintech> in (1) and (2) peer may respond with a `NetMsgType::GETBLOCKTXN` msg where we potentially send the requested txns of the block in `PeerManagerImpl::SendBlockTransactions()`
10:13 < marcofleon> seems like you all got it
10:14 < danielabrozzoni> Also `MaybeSetPeerAsAnnouncingHeaderAndIDs` takes care of switching a peer to high bandwidth mode, and if we have too many high bandwidth peers, downgrading a differernt one to low bandwidth
10:15 < marcofleon> Nice yes. Should we move on, I don't think I see any errors in the answers?
10:15 < marcofleon> The fuzz test sends SENDCMPCT messages with high_bandwidth randomly set. How many high bandwidth peers are allowed and does the fuzz harness test this limit? More generally, why would a peer choose to be high or low bandwidth?
10:16 < stringintech> there is no limit to how many peers select us as a HB peer, right?
10:16 < stringintech> i found a limit on the other side: how many peers WE can select as HB which is “3” and it is enforced when PeerManagerImpl::BlockChecked() calls`PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs()` (maintains 3 fastest peers as HB).
10:16 < stickies-v> stringintech: yeah "high bandwidth peers" is a bit ambiguous i think
10:17 < marcofleon> hmm good point. Because it's a two way street in terms of selection
10:18 < monlovesmango> I think you would prefer to be high bandwidth to remove latency?
10:18 < dzxzg> A peer doesn't choose to be high or low bandwidth, we will ask all of our peers to be low bandwidth compact block peers initially, and then in MaybeSetPeerAsAnnouncing* we'll select high bandwidth peers and send them a SENDCMPCT asking them to be a high bandwidth peers
10:18 < marcofleon> Yeah so we can have 3 max. And then I guess we could be a hb peer to all of our peers... although I'd have to confirm
10:19 < marcofleon> dzxzg: nice got it
10:19 < dzxzg> If someone has asked you to be their high-bandwidth peer, you'll send them a CMPCTBLOCK message in `NewPoWValidBlock()` before you have even completetd block validation
10:20 < marcofleon> yeah the sending the block even before validation was new to me before stickies let me know
10:20 < marcofleon> monlovesmango: yes that sounds about right
10:20 < marcofleon> the block gets sent right away so latency is very low
10:21 < monlovesmango> dzxzg: that actually makes a lot of sense
10:21 < monlovesmango> (about choosing to be high/low bandwidth)
10:22 < marcofleon> Does the fuzz harness test the 3 peer limit in MaybeSetPeerAsAnnouncingHeaderAndIDs?
10:23 < stringintech> we are setting up only 3 peers; so no?
10:24 < marcofleon> correct
10:24 < marcofleon> Would need to be more than 3 to cover that logic
10:24 < marcofleon> Compare -testdatadir and the new -fuzzcopydatadir. Why is the latter useful for performance, and why isn’t a fresh TestingSetup that mines blocks on each iteration acceptable here?
10:24 < marcofleon> stickies-v: Go off
10:25 < dzxzg> lOL
10:25 < stickies-v> Wait is that an intentional choice? The choice to connect to 3 peers seemed unrelated to the number of high bandwidth peers?
10:25 < marcofleon> No Eugene told me it wasn't intentional
10:25 < marcofleon> it's from the processmessage fuzz test I think
10:26 < stickies-v> okay makes sense
10:26 < marcofleon> But in the coverage report you can see some of the lines not hit in MaybeSet
10:26 < marcofleon> so making that more than 3 should hit it eventually
10:26 < stickies-v> yeah but then we'd probably have to add assertions that our hb accounting invariants hold
10:27 < dzxzg> yeah, it would be fine to increase it past 3, we actually don't even enforce anything related to high bandwidth on the receiving side
10:27 < monlovesmango> fresh TestingSetup would take too much time for each fuzz iteration?
10:28 < dzxzg> (https://github.com/bitcoin/bitcoin/pull/32606)
10:28 < stringintech> is this performance related (considering that mining should not take so long when in fuzzing mode) or that we want all runs to have the same initial state?
10:28 < marcofleon> monlovesmango: exactly. Having to mine a new 200 block chain every iteration is just too slow
10:29 < marcofleon> stringintech: good question, I think it's mainly for performance
10:29 < marcofleon> the determinism is there too with using the same chain. But this whole commit was mainly done to significantly speed up the test
10:30 < stickies-v> but isn't the point of `FuzzTargetOptions::init` to do that kind of expensive initialization as a one-off?
10:30 < marcofleon> dzxzg: I'll have to check that PR out
10:32 < stringintech> stickies-v: i think that's what we are doing know. mining blocks and store in a static dir in init() and then copy that static dir in each run.
10:32 < marcofleon> stickies do you mean `initialize_cmpctblock` in the harness?
10:32 < stringintech> doing now*
10:33 -!- yuvicc [~yuvicc@user/yuvicc] has quit [Quit: yuvicc]
10:33 < stickies-v> well so usually test suites have "fixtures" to do expensive one-off initialization that can be shared across tests, e.g. we have the same in our unit tests using boost
10:33 < stickies-v> it appeared to me that the `FuzzTargetOptions::init` option serves a similar purpose (but i don't know the fuzzing code well enough)
10:34 < stickies-v> if we just want to avoid mining a 200 chain for every iteration, why isn't the `.init` option enough for that?
10:35 -!- stringintech89 [~stringint@user/stringintech] has joined #bitcoin-core-pr-reviews
10:35 < marcofleon> Wait is this in fuzz.cpp? why can't I find this
10:35 < marcofleon> the fuzztargetoptions::init 
10:35 < marcofleon> oh I see nvm
10:35 < stickies-v> yes, it's in fuzz/fuzz.cpp, and it's what `.init=initialize_cmpctblock` gets passed to
10:37 < marcofleon> Yeah so that's what allows us to have the one time initialize functions in fuzz targets
10:37 < marcofleon> runs once before the iteration
10:37 < marcofleon> but what's your question?
10:38 < stickies-v> why don't we just generate the 200 blocks in the .init function and avoid copying datadirs?
10:38 -!- stringintech [~stringint@user/stringintech] has quit [Ping timeout: 250 seconds]
10:38 < stickies-v> also don't mean to derail the review club with my lack of fuzzing knowledge so we can move on from this anytime
10:38 < marcofleon> I think beacuse the test is stateful
10:39 < stringintech89> does each run alter its data dir?
10:39 < marcofleon> the testingsetup is modified in the iteration
10:40 < marcofleon> Yeah I think so, usually the testing setup stuff would be done in the init as a one off. But for this test specifically, its done in the iteration which is unusual
10:40 < stickies-v> aha, okay, that's unfortunate
10:40 < dzxzg> Is the problem that blocks are being mined and then announced?
10:41 < dzxzg> Maybe there is an obvious reason why this is dumb, but it really seems like there should be a way to have a memory-only chainstate object, and then we could just reset to that at the end of each fuzz epoch(not sure if epoch is the right word)
10:41 < dzxzg> That would also make this fuzz test faster by avoiding hitting the disk at all.
10:42 < dzxzg> Or maybe it's not a bad idea, just easier said than done to implement that
10:42 < marcofleon> My first question when I saw this test was why can't we do this in memory. And Niklas was saying its just the validation code doesn't allow it. We need to do some refactoring
10:43 < stringintech89> just asking out of curiosity: wouldn't testing memory only, reduce coverage?
10:43 < dzxzg> marcofleon: Figures 😿
10:43 < marcofleon> This datadir commit is something I need to think about more too. For now let's move on. I think the approach might be okay though
10:43 < marcofleon> Look at create_block in the harness. How many transactions do the generated blocks contain, and where do they come from? What compact block scenarios might be missed with only a few transactions in a block?
10:44 < marcofleon> stringintech89: yeah it would, by quite a bit I believe
10:44 < stringintech89> thanks
10:44 < dzxzg> stringintech89: It would reduce coverage of this particular fuzz test, but that coverage is redundant I think this test should just fuzz compact block logic which doesn't do anything special with the disk, I think existing validation fuzzers should cover that stuff
10:45 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has joined #bitcoin-core-pr-reviews
10:45 < dzxzg> (IMO)
10:45 < stringintech89> ah makes sense! thanks
10:46 < stringintech89> for the next question: max 3 txns: coinbase, at most one from mempool, at most one not in mempool. was wondering if including more than one from mempool (or more than one not in mempool) would cover different code paths and lead to more coverage.
10:46 < monlovesmango> it seems like create_block only includes 2 txs max? one from mempool and other adhoc created
10:46 < monlovesmango> oh duh, forgot coinbase :)
10:46 < marcofleon> haha yes coinbase too. 1-3 txs
10:47 < dzxzg> Why not more?
10:48 < marcofleon> I think crypt-iQ was saying he might change that part of the test a bit
10:49 < marcofleon> Or just generally improve it, I don't see why there couldn't be some more
10:49 < dzxzg> marcofleon: Makes sense, thanks
10:50 < marcofleon> I'm gonna switch question 6 and 7 for now in the interest of time if that's okay. Becauase i like 7 better :)
10:50 < marcofleon> Commit ed813c4 sorts m_dirty_blockindex by block hash instead of pointer address. What non-determinism does this fix? The author notes this slows production code for no production benefit. Why can’t EnableFuzzDeterminism() be used here? How do you think this non-determinism should be best handled (if not the way the PR currently does)?
10:50 < monlovesmango> what compact block scenarios might be missed with this low tx count?
10:50 < dzxzg> the second half of question 5, this doesn't exercise some of the prefill index stuff, where prefills have an index counting from the last prefill
10:50 < dzxzg> for example
10:51 < marcofleon> oh yes! My bad
10:51 < marcofleon> I was also thinking of shortid collisions
10:51 < dzxzg> oh yeah that too
10:51 < monlovesmango> dzxzg: marcofleon: great answers thanks :)
10:52 < dzxzg> We definitely want to have coverage of shortid collisions!
10:54 < stringintech89> for question 7: it tries to make the order we write changes to block index db deterministic
10:55 < stringintech89> and if we want to include the new sort in the set type itself, EnableFuzzDeterminism() does not work and we would need macros perhaps.
10:55 < monlovesmango> why does block index db need to be deterministic?
10:56 < monlovesmango> or why do the writes need to deterministic?
10:56 < marcofleon> Yeah basically the iteration order won't be deterministic with just a normal std::set which orders based on memory address. so it's changed to block hash to make it the same order every time
10:57 < stringintech89> but as marcofleon mentioned on the PR page, maybe we can include a runtime sorting logic just when fuzzing; then EnableFuzzDeterminism() can be used perhaps
10:57 < eugenesiegel> hi, sorry I thought this review club was at a different time. runtime sorting doesn't work because the std::sort call itself will be non-deterministic
10:58 < eugenesiegel> different executions will have different memory addresses so the compare function will be called a different amount of times
10:59 < marcofleon> Oh that makes sense. so it doesn't actually solve the non-determinism
10:59 < dzxzg> Just drawing it out a bit more to make sure I understand why we care if it's in the same order every run, we write from the set of dirty block indexes in the order of the set, and if we don't write blockindexes in the same order every time, we would have different behavior running the same seed two times?
10:59 < marcofleon> also monlovesmango: I think it's just for general stability
11:00 < marcofleon> in gneral fuzz tests should be as deterministic as possible. But not sure if it's worth it in this case, because this non-determinism might not super impactful?
11:00 < monlovesmango> ok. so just for my understanding is there anything explicity comparing this between executions?
11:00 < monlovesmango> ohhh ok so when there is a failure it can be preproduced, got it
11:01 < eugenesiegel> dzxzg: we won't technically have different behavior since iirc the db is a key-value store. The issue is that the fuzzer will think it's gaining (or losing) coverage when nothing has actually happened
11:01 < monlovesmango> reproduced*
11:01 < marcofleon> But I could be wrong there, it could be for reproducing a potential crash, this would get in the way of reproducing it
11:01 < dzxzg> I wonder what the perf cost is to normal node running of adding the pointer accesses
11:01 < marcofleon> okay we've reached the end, thank you for coming! we can of course continue discussing
11:01 < marcofleon> #endmeeting
11:01 < corebot`> marcofleon: Meeting ended at 2025-10-08T18:01+0000
11:01 < corebot`> marcofleon: Raw log: https://achow101.com/ircmeetings/2025/bitcoin-core-pr-reviews.2025-10-08_17_00.log.json
11:01 < corebot`> marcofleon: Formatted log: https://achow101.com/ircmeetings/2025/bitcoin-core-pr-reviews.2025-10-08_17_00.log.html
11:01 < corebot`> marcofleon: Minutes: https://achow101.com/ircmeetings/2025/bitcoin-core-pr-reviews.2025-10-08_17_00.html
11:02 < dzxzg> eugenesiegel: Makes sense, thanks
11:02 < monlovesmango> thanks marcofleon and all!
11:02 < dzxzg> Thanks marcofleon!
11:02 < monlovesmango> I did have another question if anyone has time to answer
11:02 < eugenesiegel> monlovesmango: I'm still around to answer any questions
11:02 < monlovesmango> for this code chunk https://github.com/Crypt-iQ/bitcoin/blob/ed813c48f826d083becf93c741b483774c850c86/src/test/fuzz/cmpctblock.cpp#L308C16-L320C18
11:02 < stringintech89> Thank you marcofleon
11:03 < monlovesmango> why is the loop not to i<=num_txs?
11:04 < dzxzg> that would go one past the end of the block
11:04 < dzxzg> the reason it starts at i = 1 is to skip the coinbase
11:04 < eugenesiegel> yup, what dzxzg said
11:04 < monlovesmango> dzxzg: thanks that makes sense 
11:05 < monlovesmango> easy question :)
11:07 < marcofleon> wait so I can understand the `m_dirty_blockindex` determinism more
11:07 < marcofleon> It's not about the order it's written to leveldb then? 
11:08 < marcofleon> wait nvm I think I just reread your message, you're saying std::sort itself is non deterministic
11:08 < marcofleon> I thought it said std::set for a second
11:11 < eugenesiegel> sorry, I think my answer was confusing. if there is a different order written to leveldb, the internal MemTable is non-deterministic, but since these are key-value pairs it shouldn't? matter on-disk
11:14 < eugenesiegel> If we insert into `m_dirty_blockindex` without sorting on insert, then `m_dirty_blockindex` is in random order. this means the sort function will call whatever std::less function a variable number of times each run for the same input. I'm completely ok with removing the commit
11:15 -!- alvroble [~alvroble@user/alvroble] has quit [Quit: Client closed]
11:16 < stringintech89> eugenesiegel: can you please elaborate a bit on this "the issue is that the fuzzer will think it's gaining (or losing) coverage when nothing has actually happened"
11:19 < marcofleon> eugenesiegel: thanks I got it, that makes more sense. I'm gonna play around with the determinism script a bit and see what comes up
11:20 < eugenesiegel> stringintech89: let's say we removed the `m_dirty_blockindex` commit, meaning the harness is now non-deterministic. if the fuzzer is given an input that calls WriteBatchSync and modifies one bit of the input that's uninteresting (doesn't gain or lose coverage), then because the harness is non-deterministic, the fuzzer may actually see an increase
11:20 < eugenesiegel> in the coverage counters and think it's due to this one bit that was modified
11:23 -!- stringintech [~stringint@user/stringintech] has joined #bitcoin-core-pr-reviews
11:25 < stringintech> eugenesiegel: ahh thank you! i think i got it (though not so clear how fuzzer calculates the coverage). will read more on it and ask more questions if still not clear.
11:26 < sipa> stringintech: when you compile with fuzzing, the compiler inserts instrumentation for tracking branch and other coverage
11:26 < sipa> which the fuzzer library then uses to guide the mutations it makes, and the corpus it keeps
11:27 -!- stringintech89 [~stringint@user/stringintech] has quit [Ping timeout: 250 seconds]
11:28 -!- stringintech66 [~stringint@user/stringintech] has joined #bitcoin-core-pr-reviews
11:30 -!- stringintech [~stringint@user/stringintech] has quit [Quit: Client closed]
11:31 < eugenesiegel> stringintech: This might be a bit out-dated, but I found this AFL writeup very helpful when learning how coverage feedback with fuzzing worked: https://lcamtuf.coredump.cx/afl/technical_details.txt
11:35 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has quit [Quit: Client closed]
11:35 -!- VonNaturAustreVe [~natur@user/vonnaturaustreve] has joined #bitcoin-core-pr-reviews
11:37 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has joined #bitcoin-core-pr-reviews
11:37 < stringintech66> thanks a lot sipa!
11:37 < stringintech66> so in the example eugenesiegel explained, for the two given inputs that have the same coverage of the code paths we are interested in, fuzzer might see different coverage score since different amount of sorting might happen...
11:38 < stringintech66> thanks a lot for the link eugenesiegel! i will definitely check it out
11:59 -!- stringintech66 [~stringint@user/stringintech] has quit [Ping timeout: 250 seconds]
12:01 -!- ___nick___ [~quassel@82-132-213-36.dab.02.net] has quit [Quit: https://quassel-irc.org - Chat comfortably. Anywhere.]
12:03 -!- ___nick___ [~quassel@82-132-213-36.dab.02.net] has joined #bitcoin-core-pr-reviews
12:04 -!- ___nick___ [~quassel@82-132-213-36.dab.02.net] has quit [Client Quit]
12:04 -!- stringintech [~stringint@user/stringintech] has joined #bitcoin-core-pr-reviews
12:06 -!- ___nick___ [~quassel@82-132-213-36.dab.02.net] has joined #bitcoin-core-pr-reviews
12:16 -!- Guest35 [~Guest35@140.174.66.131] has joined #bitcoin-core-pr-reviews
12:16 -!- stringintech [~stringint@user/stringintech] has quit [Quit: Client closed]
12:17 -!- Guest35 [~Guest35@140.174.66.131] has quit [Client Quit]
12:29 -!- Talkless [~Talkless@138.199.6.197] has quit [Quit: Konversation terminated!]
13:01 -!- ne0h_ [~natur@2804:14c:65d7:8e6c:6e7c:5b3c:3fd0:b0d2] has joined #bitcoin-core-pr-reviews
13:04 -!- ___nick___ [~quassel@82-132-213-36.dab.02.net] has quit [Ping timeout: 256 seconds]
13:05 -!- VonNaturAustreVe [~natur@user/vonnaturaustreve] has quit [Ping timeout: 250 seconds]
13:22 -!- jonatack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
13:24 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has quit [Ping timeout: 250 seconds]
13:26 -!- monlovesmango [~monlovesm@37.19.200.166] has quit [Ping timeout: 256 seconds]
13:42 -!- ne0h_ [~natur@2804:14c:65d7:8e6c:6e7c:5b3c:3fd0:b0d2] has quit [Quit: Leaving]
14:03 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has joined #bitcoin-core-pr-reviews
14:03 -!- jon_atack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
14:05 -!- jonatack [~jonatack@user/jonatack] has quit [Ping timeout: 264 seconds]
14:26 -!- jonatack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
14:28 -!- jon_atack [~jonatack@user/jonatack] has quit [Ping timeout: 265 seconds]
14:33 -!- eugenesiegel [~eugenesie@user/eugenesiegel] has quit [Quit: Client closed]
15:05 -!- grettke [~grettke@syn-184-055-133-000.res.spectrum.com] has joined #bitcoin-core-pr-reviews
15:32 -!- jonatack [~jonatack@user/jonatack] has quit [Ping timeout: 256 seconds]
15:32 -!- jonatack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
15:38 -!- grettke [~grettke@syn-184-055-133-000.res.spectrum.com] has quit [Quit: grettke]
15:46 -!- jon_atack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
15:48 -!- jonatack [~jonatack@user/jonatack] has quit [Ping timeout: 265 seconds]
15:58 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Remote host closed the connection]
16:27 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
16:30 -!- rszarka [~szarka@2603:3003:4eac:100:f588:71f1:d358:9035] has joined #bitcoin-core-pr-reviews
16:33 -!- robszarka [~szarka@2603:3003:4eac:100:705c:7907:8e14:c12a] has quit [Ping timeout: 260 seconds]
16:40 -!- jonatack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
16:42 -!- jon_atack [~jonatack@user/jonatack] has quit [Ping timeout: 265 seconds]
16:53 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Remote host closed the connection]
17:08 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
17:09 -!- jon_atack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
17:11 -!- jonatack [~jonatack@user/jonatack] has quit [Ping timeout: 256 seconds]
17:17 -!- jon_atack [~jonatack@user/jonatack] has quit [Ping timeout: 264 seconds]