The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• ci: Run ci configs on cirrus #19179 ([WIP RFC DONOTMERGE] ci: Run ci configs on cirrus by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

FWIW: The added valgrind Travis fuzz job is twice as fast as the slowest Travis job we're currently using, so it should be entirely safe to re-add this one from a build-time perspective :)

Pinging in reviewers of #18899 ("travis: Remove valgrind ") since that PR was locked for further peer review comments leaving the question of fuzz job removal unanswered.

Friendly ping @jamesob, @jnewbery, @promag, @sipa, @laanwj and @MarcoFalke: would you mind reviewing this related PR too? :)

I am no longer maintaining travis for Bitcoin Core, so I don't have an opinion on this. Personally, I moved my ci configs and scripts to https://github.com/MarcoFalke/btc_nightly . This config is one of many more that is run every day.

@MarcoFalke Understood! Thanks for clarifying. Could you clarify if the valgrind fuzzing job was causing any problems prior to the removal, or if it just slipped in with the rest of the valgrind removal? :)

If it was causing any issues that would be good to know for the reviewers of this PR to have the full picture. IIRC the valgrind fuzzing job has never caused any issues for me at least :)

Concept ACK. The fuzzing under valgrind job seems to complete much more quickly then the functional tests under ASAN/TSAN jobs.

It is not a specific test that causes problems, but if we start adding a new travis build for any configuration that could possibly find an issue, it is going to slow down everything in the same way.

For example we could add an alpine build (#18110) to check against musl libc, we could add a centos 7 build to check the minimum supported version of boost (#18943), we could add a *bsd run to check compilation against *bsd, we could add a build to check against the guix binaries, we could add a build to run the fuzzers on musl libc, we could ...

At some point we need to take into account the cost of a build against the value it adds.

I think as part of every pull request we should only run essential builds that, if failed, would bring the project development to a halt. For example breaking the gui build, or the --disable-wallet build would break most dev environments, either immediately or later when debugging a --disable-wallet bug via git bisect. Another example would be breaking the unit tests or functional tests. Intermittent issues are excluded from here, because it is impossible to protect against them.

If, however the build is broken for a few days on a system that is not run by developers (e.g. centos 7) or alpine linux, it would not bring the development to a halt.

If more builds are added to travis, then someone needs to pay for them. Personally, I think the value we get out of travis is not worth the money we spend on them. Partly because they fail to address our issues like #18868 .

Whoever is going to renew (or not renew) the contract for the next 12 months should take this into account.

It is not a specific test that causes problems, but if we start adding a new travis build for any configuration that could possibly find an issue, it is going to slow down everything in the same way.

For example we could add an alpine build (#18110) to check against musl libc, we could add a centos 7 build to check the minimum supported version of boost (#18943), we could add a *bsd run to check compilation against *bsd, we could add a build to check against the guix binaries, we could add a build to run the fuzzers on musl libc, we could ...

That is true, but note that not all issues are equally important for protecting our user's funds :)

The now removed valgrind fuzzing job helps us catch security issues whereas the examples you quote would help us catch non-security issues.

(Meta comment about our peer review process: I fully understand the frustration with Travis but TBH I think it was a mistake to lock #18899 immediately after merge without answering the question that was raised during peer review about the surprising fuzzing job removal.)

Rebased :)

(note: needs rebase on 37ae687 before merge)

@MarcoFalke

Thanks! Now rebased :)