< wumpus> jnewbery: yes, 15141 looks pretty much ready
< wumpus> jamesob: ok with me
< fanquake> jamesob / wumpus I've created the two. Will add related issues/card etc.
< wumpus> fanquake: oh that's awesome!
< fanquake> Sounds like james is going to split 15606 up quite a bit, so can dump all that in there shortly
< fanquake> ryanofsky if you've got PRs / issues you want added to https://github.com/bitcoin/bitcoin/projects/10 just dump them in here
< wumpus> how is 'shutting down' even an error
< wumpus> I guess, because it shuts down unexpectedly
< wumpus> the thread doesn't exit in 5 seconds-should be plenty of time, normally, though it's a slow system
< wumpus> re: #15141 "ValidationInvalidReason::TX_PREMATURE_SPEND is non-standard in AcceptToMemoryPoolWorker (two occurences) but invalid in Consensus::CheckTxInputs" this one is pretty weird isn't it?
< gribble> https://github.com/bitcoin/bitcoin/issues/15141 | Rewrite DoS interface between validation and net_processing by sdaftuar · Pull Request #15141 · bitcoin/bitcoin · GitHub
< wumpus> how are things looking for 0.18.0 final today?
< fanquake> wumpus Looking ok. Discussion in #15555 has dried up a bit. #15665 is still open, but no more reports of it occurring.
< gribble> https://github.com/bitcoin/bitcoin/issues/15555 | v0.18.0 testing · Issue #15555 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/15665 | 0.18.0 rc2 CPU spike in thread bitcoin-opencon · Issue #15665 · bitcoin/bitcoin · GitHub
< fanquake> Release is currently only 1 month "overdue" according to #14438 :p
< gribble> https://github.com/bitcoin/bitcoin/issues/14438 | Release schedule for 0.18.0 · Issue #14438 · bitcoin/bitcoin · GitHub
< jamesob> fanquake: thanks!
< fanquake> jamesob np
< wumpus> fanquake: looks like the release notes issues were also fixed, time to tag then !
< fanquake> wumpus \o/
< bitcoin-git> [bitcoin] laanwj pushed 1 commit to 0.18: https://github.com/bitcoin/bitcoin/compare/825ecb5758f6...2472733a24a9
< bitcoin-git> bitcoin/0.18 2472733 Wladimir J. van der Laan: build: Bump version to -final
< bitcoin-git> [bitcoin] laanwj pushed tag v0.18.0: https://github.com/bitcoin/bitcoin/compare/v0.18.0
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/ce6762030f81...45d8b7177851
< bitcoin-git> bitcoin/master 8728a66 John Newbery: [tests] fix block time in feature_pruning.py
< bitcoin-git> bitcoin/master fafb55e MarcoFalke: [qa] test_runner: Move feature_pruning to base tests
< bitcoin-git> bitcoin/master 45d8b71 MarcoFalke: Merge #15696: [qa] test_runner: Move feature_pruning to base tests
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15696: [qa] test_runner: Move feature_pruning to base tests (master...1904-qaPruning) https://github.com/bitcoin/bitcoin/pull/15696
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/45d8b7177851...10ed4dff2447
< bitcoin-git> bitcoin/master 151f3e9 Russell Yanofsky: Add settings merge test to prevent regresssions
< bitcoin-git> bitcoin/master 10ed4df MarcoFalke: Merge #15869: Add settings merge test to prevent regresssions
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15869: Add settings merge test to prevent regresssions (master...pr/testset) https://github.com/bitcoin/bitcoin/pull/15869
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #15926: tinyformat: Add format_no_throw (master...1904-tinyformatNoThrow) https://github.com/bitcoin/bitcoin/pull/15926
< bitcoin-git> [bitcoin] MarcoFalke pushed 6 commits to master: https://github.com/bitcoin/bitcoin/compare/10ed4dff2447...2c35fe623813
< bitcoin-git> bitcoin/master 188ca75 James O'Beirne: disable HAVE_THREAD_LOCAL on unreliable platforms
< bitcoin-git> bitcoin/master ae5f2b6 James O'Beirne: threads: introduce util/threadnames, refactor thread naming
< bitcoin-git> bitcoin/master ddd95cc James O'Beirne: tests: add threadutil tests
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15849: Thread names in logs and deadlock debug tools (master...2018-05-threadnames-take-2) https://github.com/bitcoin/bitcoin/pull/15849
< instagibbs> MarcoFalke, re 15881 what about having it set via parameter, and specific build in CI runs with it as mainnet default?
< instagibbs> I realize there are alrady too many cooks
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #15858: Faster tests with topological mempool rpc sync 🚀 (master...1904-qaSync) https://github.com/bitcoin/bitcoin/pull/15858
< wumpus> what was wrong with the #15858 approach? I liked keeping the change contained to the tests only
< gribble> https://github.com/bitcoin/bitcoin/issues/15858 | Faster tests with topological mempool rpc sync 🚀 by MarcoFalke · Pull Request #15858 · bitcoin/bitcoin · GitHub
< wumpus> confused why it got closed
< bitcoin-git> [bitcoin] jnewbery opened pull request #15927: [tests] log thread names by default in functional tests (master...2019-04-log-threads-tests) https://github.com/bitcoin/bitcoin/pull/15927
< jnewbery> wumpus: I prefer 15881 since it's a smaller deviation from what happens for tx propogation on mainnet
< luke-jr> #define stacktop(i) (stack.at(stack.size()+(i)))
< luke-jr> LLVM likes to cast i to unsigned and do the addition with an overflow :/
< luke-jr> should we just cast stack.size() to ssize_t or something else?
< harding> wumpus: FYI, 0.18.0 release PR for BitcoinCore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/649
< gwillen> it's not really up to LLVM, the C integer promotion rules are fixed
< gwillen> it should depend only on the integer types involved
< gwillen> unfortunately they are extremely arcane
< sipa> can we do a search replace stacktop(-i) -> stacktop(i) and then use (stack.at(stack.size()-(i)))
< gwillen> it seems like the offending rule is "If the operand that has unsigned integer type has rank greater than or equal to the rank of the type of the other operand, the operand with signed integer type is converted to the type of the operand with unsigned integer type."
< gwillen> so if you do math with a size_t (64 bit unsigned) and an int (32-bit signed) you get flattened
< luke-jr> sipa: if we do that, let's rename stacktop
< gwillen> it's a weird rule.
< gwillen> but casting to ssize_t should solve it?
< sipa> *(stack.rbegin() + i) also works :p
< luke-jr> + -i ?
< sipa> with positive i, i mean
< sipa> oh, no; *(stack.rbegin() + (i - 1))
< luke-jr> positive i is a hazard
< sipa> why?
< luke-jr> too easy to accidentally merge or backport with an invisible error
< sipa> that's fair
< gwillen> this seems like something you could check statically, if it's generally a constant
< luke-jr> gwillen: it's not always a constant
< luke-jr> anyway, keeping the negative i should be harmless
< gwillen> luke-jr: how did it come to pass that this is coming up -- did core change, or did LLVM change?
< luke-jr> gwillen: -fsanitize=undefined
< luke-jr> (apparently while we have a Travis job for it, we also have a suppressions file to hide all our existing issues!)
< gwillen> that's odd... there shouldn't be anything undefined about that, should there? Does it cite chapter and verse?
< gwillen> I can imagine it getting the wrong answer, but not being undefined
< gwillen> and it sounds like it's not in fact getting the wrong answer in practice or we'd have hit this before?
< luke-jr> gwillen: overflows are undefined behaviour
< gwillen> not unsigned overflows
< gwillen> only signed
< gwillen> if it gets converted to unsigned first it should be well-defined
< luke-jr> UBSan says unsigned are undefined too <.<
< gwillen> maybe that's why it was suppressed...
< gwillen> can you pastebin the ubsan complaint?
< sipa> luke-jr: by default ubsan does not warn on unsigned overflow, but there is an optional flag to enable them
< sipa> they're not undefined behavior, but a common red flag
< luke-jr> gwillen: it scrolled out of my terminal - and I'm not entirely sure how I triggered it, since I wasn't running on mainnet ☺
< luke-jr> sipa: odd, I didn't explicitly set any flag
< sipa> the core build system enables it afaik
< luke-jr> ah
< luke-jr> is the cast from negative to unsigned also well-defined?
< luke-jr> script/interpreter.cpp:922:42: runtime error: unsigned integer overflow: 2 + 18446744073709551615 cannot be represented in type 'unsigned long'
< luke-jr> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior script/interpreter.cpp:922:42 in
< luke-jr> gwillen: ^
< sipa> luke-jr: cast from signed to unsigned is well defined
< sipa> the other way isn't afaik
< * luke-jr> typically avoids answering these questions by being explicit on anything that could reasonably be undefined :p
< gwillen> yeah so it seems like we are in one of the corners of integer promotion that is weird but not bad
< gwillen> wherein C promotes from signed to unsigned in a way that creates apparent integer overflow that actually gives the correct answer consistently
< gwillen> casting to ssize_t will probably get rid of the weirdness but will cause undefined behavior (I think) if the stack has more than 2 billion elements, which I expect is not a concern
< gwillen> actually if size_t is 64 bit it would need to be a lot more than that
< luke-jr> gwillen: ssize_t isn't guaranteed to be large enough for all sizes?
< gmaxwell> gwillen: we are misusing sanitization tools
< gmaxwell> and using suppression files to cover up the misuse
< gmaxwell> and then probably just giving up on making useful changes, to cover up the difficulty of updating the suppressions.
< gmaxwell> there are a ton of sanitizers which are available which trigger on behavior that is sometimes (even only rarely) a mistake, and turning them on means either (1) you leave them producing false positives which you ignore, causing you to miss real reports, (2) go change the code in ways that silence them, often lowering code quality or even adding bugs, (3) suppress them, and then deal with the
< gmaxwell> returning every time you add a new instance of the same idiom or refactor something
< gmaxwell> In commercial tools like coverity these concerns are addressed by not emitting warnings for anything that has a high false positive rate (even if that misses some true positives), and by havinga nice database that lets you track suppressions, keep notes on them, and which manages to survive refactoring most of the time.
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #15881: net: Send txs without delay on regtest (master...1904-txRelayRegtest) https://github.com/bitcoin/bitcoin/pull/15881
< gwillen> gmaxwell: *nod*, that all makes sense