< bitcoin-git>
[bitcoin] instagibbs opened pull request #19649: Restore test case for p2p transaction blinding (master...p2p_segwit_reject) https://github.com/bitcoin/bitcoin/pull/19649
< bitcoin-git>
[bitcoin] kallewoof reopened pull request #14582: wallet: always do avoid partial spends if fees are within a specified range (master...181026-try-avoidpartialspends) https://github.com/bitcoin/bitcoin/pull/14582
< jnewbery>
wumpus MarcoFalke fanquake: I think #18991 may be ready for merge. It's had a ton of review and has three ACKs on the most recent branch (and a bunch more ACKs/concept ACKs on previous branches)
< aj>
hebasto: why do you think adding GUARDED_BY is a design change? it's just those members existed before bitcoin source had thread safety annotations, as far as i know?
< hebasto>
aj: it changes class invariants
< aj>
hebasto: no it doesn't? (or: i don't see how?)
< aj>
hebasto: (i'd say that it *documents* class invariants, just like the rest of your pr)
< hebasto>
aj: ok, if new GUARDED_BY will not cause to introduce new need to acquire a lock somewhere
< aj>
hebasto: seemed to work fine for me, fwiw
< aj>
hebasto: (if it did demand a lock was acquired somewhere, i'd expect that to be a bug worth fixing anyway)
< hebasto>
btw, `AssertLock{Not}Held` were my tools to transit `CTxMemPool::cs` to `RecursiveMutex` in #19306
< hebasto>
aj: sudden run-time error that were not caught by compile-time thread safety analysis could be (almost) eliminated with full annotation coverage
< aj>
hebasto: it crossed my mind that maybe DEBUG_LOCKORDER should check for accidental recursive locking attempts with Mutex (or even RecursiveMutex if it just logged a warning on first occurence or so?)
< aj>
hebasto: hmm, "EX_LOCKS_REQ(!cs)" complaints are only warnings, not errors?
< hebasto>
we could always pass -Werror, no?
< aj>
hebasto: i thought we were?
< aj>
no, apparently not? did i do configure wrong?
< aj>
ah, missed --enable-werror i guess?
< hebasto>
by default `--enable-werror=no`
< aj>
yeah, there's already '-Werror=thread-safety' by default
< aj>
or not?
< hebasto>
you should add `--enable-werror`
< aj>
yeah, have done so. weird i haven't felt forced to before
< aj>
hebasto: hmm, apart from a missing EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet) in BlockUntilSyncedToCurrentChain(), it seems possible to annotate AssertLockNotHeldInternal with EXCLUSIVE_LOCKS_REQUIRED(!*cs) (have to templatize it to avoid "dereferencing" a void* too of course)