< 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.
< 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
< 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
< 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>
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/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?
< 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
< 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