< phantomcircuit> sipa, can you find a new host for gods sake, that one is still randomly banning half the internet
< sipa> hmm?
< sipa> which one?
< phantomcircuit> bitcoin.sipa.be
< sipa> what's wrong with it?
< phantomcircuit> sipa, it blocks basically any ip that's shared by more than 1 person, including virtually every ipv4 address used by att mobile
< phantomcircuit> sipa, is there some kind of firewall in their settings or something
< sipa> phantomcircuit: and what do you think is blocking me?
< phantomcircuit> sipa, is blocking you?
< gmaxwell> phantomcircuit: you've got it backwards I think.
< gmaxwell> The problem is that the seenode domain resolves to "bad hosts" from time to time.
< gmaxwell> So then this causes shitbag (technical term) ISPs like AT&T mobile to ban the domain.
< gmaxwell> We've fretted about this before and it was suggested that the IPs returned by the seednodes be obfscuated somehow, but unfortunately many kinds of obfscuation will result in recursive resolvers dropping those results.
< gmaxwell> (e.g. it's not uncommon for recursive resolvers to drop unroutable IP prefixes.)
< phantomcircuit> gmaxwell, im talking about bitcoin.sipa.be, which isn't the thing resolving to weird stuff
< sipa> oh
< sipa> but maybe by being similar to seed.bitcoin.sipa.be ?
< gmaxwell> phantomcircuit: I think they block access to the anything at the IP of the nameserver serving it, because they think it's a botnet fast flux control.
< phantomcircuit> gmaxwell, hmm interesting
< booyah> fyi going to "http://bitcoin.sipa.be/depths.png" --> "Warning: Unsafe Website Blocked!" (I was redirected to http://warn.recursive.dnsbycomodo.com/?host=bitcoin.sipa.be )
< booyah> and same thing for seed.
< fanquake> wumpus / sipa can you block sinette on GH. Spamming, and by the look of their profile doing in on multiple repos.
< wumpus> sure
< wumpus> done
< pbase> what should be the starting point to start understanding the bitcoin-core code?
< wumpus> pbase: what part are you interested in?
< wumpus> validation, the wallet, etc, that already provides a place to start looking
< pbase> wumpus, i intend to create a decentralized identity system from it
< pbase> wumpus, so guess all the components
< pbase> will have to forgo anonymity
< pbase> Will it be a good idea to start from the first version of the code?
< wumpus> pbase: not necessarily, newer versions tend to have more code, but also cleaned up a lot of the code, made better abstractions, and so on
< wumpus> and better comments (hopefully)
< promag> what's the deal with travis failures?
< wumpus> I don't know…
< promag> "SC2236: Use -z instead of ! -n"
< wumpus> it's not failing on master is it?
< wumpus> not getting mails at least
< promag> update to shellcheck?
< promag> doesn't look like
< wumpus> please not another linter issue I can't survive this...
< promag> keep calm and fix linter
< wumpus> rm -rf /linters
< promag> path not found
< promag> well honestly I like linters
< wumpus> everyone does but me
< promag> most of the time they play on our side
< promag> on my system it's "version: 0.4.7", on travis 0.6.0
< promag> weird, that's not on bionic
< promag> where does that come from?
< wumpus> I like static checking that finds bugs, or rules out classes of bugs. What I don't like is test failures because of what are essentially style suggestions.
< wumpus> ! -n is perfectly fine, yes -z is shorter no shit no that's not something that needs to be fixed
< gribble> Error: "-n" is not a valid command.
< promag> ok, upgraded shellcheck to 0.6.1 and got that error
< promag> pushing fix
< wumpus> I think SC2236 needs to be added to 'shellcheck -e ...'. though if they change the shellcheck version arbitrarily it'd be better to list the cases that should be checked inclusively, instead of excluding certain checks
< promag> yes
< promag> #15164
< gribble> https://github.com/bitcoin/bitcoin/issues/15164 | qa: Ignore shellcheck warning SC2236 by promag · Pull Request #15164 · bitcoin/bitcoin · GitHub
< wumpus> I'm confused by #15104, how can adding unit tests *decrease* test coverage
< gribble> https://github.com/bitcoin/bitcoin/issues/15104 | Tests: Add unit testing for the CompressScript function by mmachicao · Pull Request #15104 · bitcoin/bitcoin · GitHub
< wumpus> good
< wumpus> $ ../devtools/github-merge.py 15164
< wumpus> Warning: unable to retrieve pull information from github: HTTP Error 403: Forbidden
< wumpus> this happens a lot lately, did github change the request limits?
< wumpus> --might make sense to add reporting of the detailed http error to gh-merge
< promag> do you exceed 5000req/hr?
< wumpus> yep... "message":"API rate limit exceeded for X.X.X.X"
< wumpus> eh definitely not
< promag> it counts all your requests
< promag> "all OAuth applications authorized by a user share the same quota of 5000 requests per hour when they authenticate with different tokens owned by the same user."
< wumpus> this is for *authenticated requests*? FWIW, that script uses unauthenticated ones
< promag> then it's 60 requests per hour
< wumpus> that's still somewhat unlikely but of a more realistic magnitude
< promag> but it could use your PAT?
< promag> it's for retrieve_pr_info?
< wumpus> req = Request("https://api.github.com/repos/"+repo+"/pulls/"+pull)
< wumpus> yes
< wumpus> hi testbot_
< wumpus> just going to patch it locally for now to put in my token
< wumpus> that works !
< promag> \o/
< promag> wumpus: I don't think we should encourage that
< promag> .. git config for secrets
< promag> I'd say something like "set environment variable GITHUB_TOKEN with Github Personal Access Token to overcome API rate limits"
< wumpus> environment variables are not better!
< wumpus> FWIW what i have myself is
< wumpus> [include]
< wumpus> path = ~/.gitsecrets
< promag> wumpus: I don't mean to have the envvar in the shell
< wumpus> but I thought that would be overly pedantic to suggest in the documentation, this is a token without privileges
< promag> is it?
< wumpus> yes
< wumpus> where else would you have environment variables than in the shell? it's often possible to get the envvars in 'ps' output on a shared system, at least configuration files can be set with appropriate permissions
< promag> I mean PAT=(securely get my pat) ./gh-merge
< wumpus> then agian, I don't feel like having an argument about this, closing that PR if this is controversial
< promag> don't do that
< wumpus> promag: that has the same result! and do you want to copy/paste the token every time you use the script?
< wumpus> also that exposes it to shoulder surfers
< promag> PAT=(run-command-to-securely-get-my-pat) ./gh-merge is unsecure? anyway I'm too dumb in this regard.. :P
< wumpus> I thought about suggesting to add the token in the *local* git config, might be less likely to accidentally check that in
< wumpus> (e.g. it's part of the .git directory so I don't think you even can)
< wumpus> for me, though, it's much more useful to have it globally available
< wumpus> (I use the tool for more repos)
< promag> err nevermind that, that's for git via http iiuc
< promag> moving forward X)
< wumpus> promag: I don't know, I might be confused too, but isn't this what 'ps e' is?
< promag> mother-of-god
< promag> back to linux for dummies
< promag> wumpus: since you utACK on #14941 do you mind weight about ryanofsky suggestion?
< gribble> https://github.com/bitcoin/bitcoin/issues/14941 | rpc: Make unloadwallet wait for complete wallet unload by promag · Pull Request #14941 · bitcoin/bitcoin · GitHub
< wumpus> promag: will take a look
< promag> ty
< wumpus> promag: he makes a valid suggestion IMO, no strong opinion on it, I'd slightly prefer simpler code (I don't think it's too bad that the last RPC wallet call, if it's still in progress, has to wait too -- I think it is unlikely in practice that that happens at the same time)
< promag> wumpus: yap, I agree with you all
< promag> if that turns to be a problem then it can be avoided
< MarcoFalke> [09:44] <wumpus> I'm confused by #15104, how can adding unit tests *decrease* test coverage
< gribble> https://github.com/bitcoin/bitcoin/issues/15104 | Tests: Add unit testing for the CompressScript function by mmachicao · Pull Request #15104 · bitcoin/bitcoin · GitHub
< MarcoFalke> They are not deterministic right now, so you get different coverage each run
< wumpus> MarcoFalke: right, so the interpretation would be: the gain in test coverage, if any, is within the random margin
< MarcoFalke> Yeah, to see where coverage increases you'd have to manually discard all files which you think shouldn't change
< MarcoFalke> So compare
< gribble> https://github.com/bitcoin/bitcoin/issues/54 | do not create "Your Address" account when creating a new wallet by tcatm · Pull Request #54 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/54 | do not create "Your Address" account when creating a new wallet by tcatm · Pull Request #54 · bitcoin/bitcoin · GitHub
< MarcoFalke> Looks like coverage is already there, but I wouldn't mind additional tests if they make sense
< * luke-jr> pokes gribble
< wumpus> MarcoFalke: +1
< wumpus> also don't think those few extra unit tests make test_bitcoin take significantly more time
< gmaxwell> Re: coverage, is the coverage analysis excluding the tests themselves? If they aren't it's totally unsurprising that adding a test would reduce coverage.
< MarcoFalke> They are not excluded, but since they are run, they are covered
< gmaxwell> MarcoFalke: tests frequently contain error handling code which doesn't run (because the tests pass), which means that adding a test that mostly covers otherwise covered code can reduce coverage.
< MarcoFalke> Right. Though, we mostly use the BOOST_* macros for error handling/reporting and those system libs are excluded from coverage
< luke-jr> macros != libs; are you sure they're excluded?
< MarcoFalke> (Only folders or files in /src)
< MarcoFalke> Makefile.am:LCOV_FILTER_PATTERN=-p "/usr/include/" -p "/usr/lib/" -p "src/leveldb/" -p "src/bench/" -p "src/univalue" -p "src/crypto/ctaes" -p "src/secp256k1"
< luke-jr> MarcoFalke: yeah, but I'm not sure how it determines which file it's in
< luke-jr> with a macro, it could very well be considered part of the test file
< MarcoFalke> Ah
< MarcoFalke> Yeah, for branch coverage it does, I think.
< MarcoFalke> I couldn't really get meaningful data for branch coverage anyway. I guess the compiler is adding too many optimization branches or removes them
< MarcoFalke> For line and function coverage it doesn't matter
< gmaxwell> MarcoFalke: what do you mean you couldn't get meaningful data?
< MarcoFalke> I mean that I am too dumb to parse the html output
< MarcoFalke> bool Enabled() const has two branches
< MarcoFalke> But the compiler adds hundered or so more
< MarcoFalke> Could be some inlining of the template stuff further down, idk
< gmaxwell> MarcoFalke: yes, thats totally sensible, those are real branches that come in due to exception handling and templates.
< promag> should I include release notes in a backport?
< luke-jr> probably depends on which gets released first
< promag> makes sense
< promag> luke-jr: do you still dislike #15149?
< gribble> https://github.com/bitcoin/bitcoin/issues/15149 | gui: Show current wallet name in window title by promag · Pull Request #15149 · bitcoin/bitcoin · GitHub
< luke-jr> promag: no, the responses satisfy my concern
< promag> cool thanks
< * luke-jr> edits a strikeout into his comment
< luke-jr> I mean, I guess I dislike it, but it's not unreasonable
< luke-jr> I actually am annoyed sometimes that I can search my open browser windows, but it misses non-active tabs :P
< phantomcircuit> i've almost got the 0.7 bdb based node to 2018, it's been literally a month i think
< promag> I think hebasto can be added to the gh org?
< bitcoin-git> [bitcoin] promag closed pull request #15107: rest: Return 404 in /rest/headers if block hash does not exists (master...2019-rest-header-404) https://github.com/bitcoin/bitcoin/pull/15107
< gkrizek> ^^ GitHub IRC Service is out of its brown out until the 31st when it’s fully depreciated. I’m still working on a replacement and it’s close to done. Will def be done before the 31st.
< gwillen> meshcollider: as wallet maintainer do you have any official opinion on how much more review #14978 should have
< gribble> https://github.com/bitcoin/bitcoin/issues/14978 | Factor out PSBT utilities from RPCs for use in GUI code; related refactoring. by gwillen · Pull Request #14978 · bitcoin/bitcoin · GitHub