19:03:37 <achow101> #startmeeting 19:03:37 <core-meetingbot> Meeting started Thu Dec 15 19:03:37 2022 UTC. The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings. 19:03:37 <core-meetingbot> Available commands: action commands idea info link nick 19:03:44 <brunoerg> hi 19:04:06 <lightlike> hi 19:04:09 <achow101> #bitcoin -core-dev Meeting: achow101 aj amiti ariard b10c BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral laanwj larryruane lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos nehan NicolasDorier paveljanik petertodd 19:04:09 <achow101> phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild 19:04:24 <vasild> hi 19:04:24 <kanzure> hi 19:04:34 <achow101> Welcome to the weekly general IRC meeting 19:05:03 <LarryRuane> Hi 19:05:18 <achow101> There is one preproposed meeting topic: https://github.com/bitcoin/bitcoin/pull/25871 (bytes1440000) 19:05:38 <achow101> any last minute topics to add? 19:06:19 <achow101> Let's start with the usual 19:06:25 <achow101> #topic High priority for review 19:06:25 <core-meetingbot> topic: High priority for review 19:06:53 <achow101> https://github.com/orgs/bitcoin/projects/1 anything to add/merge/remove? 19:07:19 <Murch1> Hi 19:08:05 <achow101> and also reminder that we now have https://github.com/orgs/bitcoin/projects/5 where people can add their own PRs 19:08:52 <brunoerg> cool 19:10:19 <achow101> #topic https://github.com/bitcoin/bitcoin/pull/25871 (bytes1440000) 19:10:19 <core-meetingbot> topic: https://github.com/bitcoin/bitcoin/pull/25871 (bytes1440000) 19:10:27 <achow101> #25871 19:10:29 <gribble> https://github.com/bitcoin/bitcoin/issues/25871 | contrib: add vasild to trusted keys by vasild ÷ Pull Request #25871 ÷ bitcoin/bitcoin ÷ GitHub 19:11:07 <vasild> I have no idea what is the status of that, but I am here to answer questions, if any (a bit sleepy, though :) 19:11:10 <achow101> I think bytes1440000 was wondering what the status of that was 19:11:19 <b10c> hi 19:13:19 <achow101> I think the current status is that a few contributors are -0 or -1 19:16:18 <achow101> and the next steps would be to follow up with those contributors to see if any of the discussion has changed their opinion 19:16:54 <aureleoules> hi 19:17:36 <achow101> anything else to discuss? 19:18:26 <aureleoules> yes 19:18:44 <aureleoules> I've been experimenting with brunoerg on mutation testing and I would love your feedback 19:18:53 <vasild> achow101: who are those contributors? there are no NACKs on github, the sole -1 is "-1 on maintaining net processing" from dergoegge but "net processing" was omitted from the scope... 19:19:18 <vasild> no -0 either 19:19:34 <achow101> vasild: dergoegge fanquake and jeremyrubin I think 19:20:08 <achow101> unless I missed a followup comment, but no acks from them either 19:21:17 <_aj_> are there any PRs that are stalling due to lack of people merging, rather than lack of review or waiting-for-author/rebase? 19:22:03 <_aj_> (if so, maybe move them to the Ready for merge column in the "PR Statuses" project linked above?) 19:23:26 <achow101> _aj_: I don't think so. I've been looking at the ack counts of all of the open prs, and the highest is 2, so not particularly certain that those are ready for merging 19:24:10 <achow101> although some may have had acks until they got conflicted and needed rebasing 19:26:22 <jeremyrubin> i withdraw all my historical acks or nacks on core prs; don't think that they particularly matter and the maintainers should just do what they see fit to advance the project 19:28:05 <achow101> doesn't seem like there's much else to say on this topic 19:28:14 <achow101> #topic mutation testing (aureleoules) 19:28:15 <core-meetingbot> topic: mutation testing (aureleoules) 19:28:52 <aureleoules> So I've been experimenting with mutation testing with brunoerg 19:29:04 <aureleoules> It's a testing technique that allows to discover parts of the code that is untested 19:29:13 <achow101> anything interesting? 19:29:25 <aureleoules> So far I've built this UI https://bcm-ui.aureleoules.com/status/NotKilled 19:29:54 <aureleoules> I believe it works well but it produces a lot of data 19:30:11 <brunoerg> Yeah 19:30:18 <aureleoules> Some mutations are just belt and suspenders check that are not reachable by the users 19:30:28 <achow101> are the mutations generated automatically or by hand? 19:30:33 <vasild> "discover parts of the code that is untested" -- is that not basic code coverage? 19:30:36 <_aj_> i like that the status/Killed link has the website die in sympathy :) 19:30:47 <aureleoules> They are generated automatically using regexes but the files are choosen manually for now 19:31:14 <aureleoules> vasild: it does sometime overlap with classical test coverage but it also discovers new parts of the code that is untested 19:31:18 <_aj_> vasild: code coverage just tells you that it was executed, not that the semantics were tested 19:31:30 <lightlike> is it unit tests only, or would it only find mutations killed by functional tests? 19:31:32 <aureleoules> you can match the results with https://marcofalke.github.io/btc_cov/total.coverage/index.html for example 19:31:41 <lightlike> *also find 19:31:53 <aureleoules> _aj_: haha yes I haven't done the pagination yet, netlify doesnt like it 19:32:21 <aureleoules> lightlike: I first run the unit tests because they are faster and then the functional tests, if none crash it means the mutation has survived and a test should be added 19:32:29 <_aj_> aureleoules: what does "Fixed" mean? 19:33:04 <brunoerg> reminding that it's not a generalist C++ tool, but focused on Bitcoin Core, because we have e.g. CAmount a = 1; and most tools can't deal with it 19:33:06 <aureleoules> _aj_: it means a test was added or ignored (for now) because it is unreachable code or not important to fix 19:34:23 <aureleoules> you can mind some examples of mutations in this file for example: https://github.com/aureleoules/bticoin-core-mutuaitons/blob/master/mutator/src/mutators/std_algorithm.rs#L23 19:35:11 <achow101> it's definitely useful to have mutation testing. I think gmax was doing mutation testing for Taproot and found a couple of bugs that way. would be nice to have that over more prs 19:36:15 <aureleoules> the next step for me and bruno is to mutate the diff of pull requests to add as much test coverage and thus maybe find bugs 19:36:36 <vasild> Is that like replacing std::sort in the source code with std::stable_sort and checking if more code will be covered, compared to std::sort? 19:37:32 <aureleoules> i'm not sure what you mean by "if more code will be covered" 19:37:42 <aureleoules> but it tests that the CI should crash with std::stable_sort 19:37:55 <aureleoules> this may not be a very useful mutator after all 19:38:11 <aureleoules> std::min -> std::max is more likely to crash 19:38:28 <brunoerg> if it doesn't crash with stable_sort, it means we should use it instead? is this the purpose? 19:38:32 <vasild> I mean different code path to be executed 19:39:46 <_aj_> mutation testing is about introducing bugs into the code (by changing if (x == y) to if (x != y) and similar), and seeing if the existing tests catch the bug and report an error, or if all the tests pass 19:40:15 <vasild> ah, I see now, thanks _aj_! 19:41:12 <aureleoules> if it doesn't crash with stable_sort it means that there should be a test somewhere that checks how a vector was sorted (but I believe this is usually not tested) 19:41:27 <_aj_> int subtract(int a, int b) { return a + b; } // you can have a fuzz tester that has 100% coverage of that code, despite the obvious bug. mutation testing will tell you the tests don't care that you're return "a + b" or "a - b", which will definitely help improve the tests, and may make the bug obvious 19:41:44 <brunoerg> _aj_: perfect 19:42:00 <MacroFake> Does it parse the AST or will it also modify string literal contents? 19:42:23 <aureleoules> I doesn't parse the AST, it parses the source code with simple regexes (for now) 19:42:33 <aureleoules> And I added a check to not mutate string litterals 19:42:52 <aureleoules> the bottle neck is not the compile time it's the CI, so I don't think mutating the AST is the priority 19:43:11 <aureleoules> it* doesnt 19:44:16 <MacroFake> Does it use ccache? 19:44:23 <aureleoules> yes 19:45:17 <aureleoules> I can add as many workers to execute the mutations with a simple docker run 19:45:23 <aureleoules> the Dockerfile for the worker is here https://github.com/aureleoules/bticoin-core-mutuaitons/blob/master/docker/Dockerfile.builder 19:45:58 <aureleoules> It contains a pre-compiled bitcoin-core with ccache (thus the image is heavy) but the first compilation is fast 19:45:58 <vasild> bticoin 19:46:36 <aureleoules> vasild: yes the repo name has been mutated as well :) 19:46:52 <vasild> :-D 19:46:56 <brunoerg> lol 19:47:53 <vasild> this sounds super cool! 19:48:29 <aureleoules> thanks! 19:49:01 <aureleoules> if you have an idea on how to make it better, issues are very welcome! 19:49:33 <achow101> awesome 19:49:36 <achow101> any other topics to discuss? 19:49:47 <vasild> what about deleting random lines of code and seeing what happens? 19:50:14 <aureleoules> vasild: I have thought of that but that will produce huge amounts of data 19:50:24 <aureleoules> considering every file is 4000 lines 19:50:35 <instagibbs> aureleoules, could "score" subroutines based on importance 19:50:36 <vasild> yeah, and 99% compilation failures, I guess 19:50:47 <brunoerg> i'm working on a CLI for asmap stuff, it downloads dumps, convert dump files to human-readable ones, compare two files, and other stuff. if anyone has any idea to improve it or features it could have, that's the repo: https://github.com/brunoerg/asmapy 19:51:00 <instagibbs> i did some manual mutation testing for taproot as well, but i focused on things like sighashes and related, not the entire PR 19:51:04 <aureleoules> i could add a check to not mutate brackets 19:51:08 <instagibbs> (found a bug as well) 19:51:20 <aureleoules> but compile time failures are great because they happen quickly 19:52:08 <vasild> maybe just for the lines that are added or modified in open PRs, not for the entire code base 19:52:19 <aureleoules> instagibbs: what do you mean by "could "score" subroutines based on importance" 19:52:34 <aureleoules> vasild: yes this is planned 19:52:43 <instagibbs> files are big, but some functions are very small and very important 19:53:01 <instagibbs> triaging mutation testing coverage by consensus criticality 19:53:10 <brunoerg> we could point what interval of lines we wanna mutate 19:53:49 <aureleoules> good idea brunoerg 19:56:09 <achow101> #endmeeting