< meshcollider>
jonasschnelli: to rebuild appveyor, just make sure you're logged in with github and click Re-Build PR at the top of the page, next to Log
< jonasschnelli>
meshcollider: I don't have a restart button (logged in via GitHub)
< meshcollider>
jonasschnelli: weird, in your github settings on the Authorized OAuth Apps page, does AppVeyor have access to read org and team membership, and the bitcoin org?
< wumpus>
#14711 seems almost ready for merge for a while, really just waiting for ryanofsky to respond to empact's comment on the test
< wumpus>
promag: seems like something that can be done later, blocking the GUI a little bit when opening a file is more or less expecated
< wumpus>
(unless it takes so long it needs a progress indicator)
< promag>
wumpus: no, I kind of have that, it just doesn't show the progress
< promag>
0 -> 100%
< promag>
it's a indeterminate progress dialog
< fanquake>
promag I'll have a look again
< promag>
fanquake: hi, I'll push the progress dialog in a bit
< promag>
I'll let you know
< wumpus>
the risk of blocking the GUI thread too long is that some operating systems (as well as users) will conclude that the program crashed and terminate it
< promag>
or the user
< fanquake>
wumpus yea, in the original PR, the gui would essentially freeze, and you'd get the macOS beach ball, so a user would conclude a crash.
< promag>
someone should come up with a good text, like "Opening wallet foobar, please wait bla bla...2
< wumpus>
fanquake: right
< promag>
fanquake: I put some millisleep in the rescan loop and it is indeed bad
< promag>
this doesn't happen when opening the wallet with loadwallet RPC in the RPC console
< promag>
as the command is executed in a background thread
< promag>
*RPC server thread
< promag>
fanquake: provoostenator: added a progress indicator while the open is loading
< fanquake>
promag cool, i'll try have a look tonight
< promag_>
provoostenator: I think we should do as little as possible regarding fancy GUI and threading jiggling
< provoostenator>
As little as ppossible yes, but a blocking UI and/or missing transactions without explanation is not acceptable either.
< provoostenator>
See my new comment. However if it's too difficult, we could also just add a standard warning, if the wallet is more than N blocks old, that "this may take a while".
< provoostenator>
A progress bar plus a way to abort seems way nicer though, and you've made good progress :-)
< promag>
provoostenator: what you mean by "I don't think the progress bar actually works." ?
< provoostenator>
It just shows 100% all the time.
< wumpus>
progess bar doesn't work for things that run in the GUI thread
< wumpus>
to update the GUI it needs a running GUI event loop
< wumpus>
a long time ago there was the same problem for initialization/shutdown, which is why it was moved to a separate thread
< provoostenator>
wumpus: rescan wallet has a pretty descent progress bar in the GUI though.
< provoostenator>
(oh wait, you mean GUI thread, not GUI period)
< wumpus>
I think that runs in a separate thread that sends progress notifications to the GUI event loop thread
< wumpus>
yea
< provoostenator>
Imo it should figure out which blocks the rescan covers, because it can take a *long* time.
< provoostenator>
The logs do show this progress by the way, so it's "known" by something.
< promag>
provoostenator: the problem is that the progress notification is a global notification
< promag>
for instance, it would be problematic if there are 2 tasks reporting progress
< promag>
and while that works for dumping progress to the log, for the GUI it should be more contained
< promag>
what I mean is that interfaces::Node::loadWallet should probably return an interfaces::Activity, and this would allow to know the progress, errors etc and also to pause, cancel etc
< promag>
provoostenator: pushed, let me know
< jamesob>
do we prefer (i) more #ifdefs or (2) very light unnecessary header bloat?
< jamesob>
sipa: missed that, will give it a read. thanks for the pointer
< sipa>
one of the ideas i suggested there was to have a debug mode where the queue depth is 0, which would cause the deadlock detector to trigger on anything that would become a deadlock in case of queue overflow
< sipa>
though there are existing violations in the code
< jamesob>
sipa: so that would effectively collapse the scheduler thread into the caller's?
< sipa>
yes
< sipa>
and if that works, there should be no risk in having a limited (but bounded) queue depth to increase parallellism for taks that can meaningfully be done in the background
< jamesob>
sipa: seems like that might make it harder to find races when the deadlock detector is enabled though, no?
< sipa>
how so?
< jamesob>
you're losing the asynchronicity of the scheduler
< sipa>
it immediately causes every callsite of a scheduled task to not hold the locks that its background task will need
< sipa>
and makes the deadlock detector notice these
< sipa>
it's still possible to have two different modules with different locks that both post a background task that runs in the other module's locks... but that would be a circular dependency in the code anyway
< sipa>
i'm not saying this should be the only way of testing the scheduler
< jamesob>
sure
< sipa>
it just feels that your PR is very specialized to one instance of a background task
< jamesob>
yeah, agreed
< sipa>
and with that approach i expect we'll miss future introductions
< sipa>
hmm
< sipa>
what if we require that all calls to scheduler posts are done with no locks held?
< jamesob>
heh, that sounds like a huge refactor
< jamesob>
well maybe not huge
< sipa>
i don't think so
< sipa>
it's almost the case already
< sipa>
apart from invalidateblock
< jamesob>
sipa: talked a bit with sdaftuar and ryanofsky offline; sounds like it'd be worth trying to write the depth=0 mode and seeing where it breaks down
< sipa>
jamesob: i think now that trying to just enforce no locks when posting is better
< sipa>
you could have two modules that bothbrequire different locks, and each posts a job that needs the other's lock
< sipa>
which you don't detect with the deoth=0 thing
< sipa>
no locks at all is overkill, but just as easy to achieve (from my limited memory), and much more tight
< provoostenator>
Is there an easy way to change the wallet height directly in a wallet.dat file (to trigger a rescan upon load)?
< phantomcircuit>
sipa, wait openssl gutted a function and replaced it with a stub that didn't even try to do what the original did?
< phantomcircuit>
the fuck
< wumpus>
provoostenator: hmm maybe one of the db4.8_ tools can be used to change the concerning key/value directly?
< wumpus>
(a very crude way would be to use db_dump, edit the output, then db_load that to a new database)
< provoostenator>
The db_dump route sounds reasonable, if that's more readable then the .dat file itself. I only have to do it once, and then just keep a copy around.
< wumpus>
well it's hex...
< jamesob>
sipa: what would we do with all the GetMainSignals(). calls in, say ConnectTip, while we're holding cs_main (of which there are many per ABC call)? buffer them up?
< jamesob>
swapping out scheduler execution for an immediate blocking call sounds really easy compared to enforcing a no-lock push, and it seems like we'd get the same testing benefit
< sipa>
jamesob ok
< hebasto>
bionic used for gitian builds has apt 1.6.6ubuntu0.1 with fixed CVE-2019-3462
< jamesob>
sipa: but if you can think of an easyish way to do it, I'm happy to try
< sipa>
jamesob: we can do one first and the other later
< sipa>
both will require breaking up invalidateblock though
< wumpus>
hebasto: I'm sure it will be widely patched now; what is kind of worrying to me is all the time that this issue did exist, people that knew about it could install arbitrary packages on every debian* system
< wumpus>
hebasto: MITMing a mirror is not trivial and requires access to someone's network, but say, open wifi networks or tor exit nodes would certainly be a vector
< harding>
Don't apt packages require an approved signature in order to be installed? The linked post doesn't describe how that protection is bypassed.
< wumpus>
you're right that signing *should* prevent this, I think that's what evryone expected
< harding>
jamesob: ah, thanks! "The parent process will trust the hashes returned in the injected 201 URI Done response, and compare them with the values from the signed package manifest. Since the attacker controls the reported hashes, they can use this vulnerability to convincingly forge any package."
< hebasto>
wow ^
< wumpus>
ouch.
< sipa>
ouch.
< luke-jr>
does apt-cacher prevent this?
< wumpus>
that's hard to say, depends on whether it simply dumbly caches and forwards the response, or does its own validation
< wumpus>
A *normal* http proxy at least is not going to stop this
< wumpus>
I wonder if the apt packages are served over https
< luke-jr>
apt ships without https support
< wumpus>
bleh
< luke-jr>
I do wonder how they expect people to patch this
< luke-jr>
since updating apt implies exposing yourself
< wumpus>
apparently the flag "apt -o Acquire::http::AllowRedirect=false" prevents the issue
< luke-jr>
ah
< wumpus>
the idea is that you need to use that once for both update and upgrade, then make sure it updates your apt
< luke-jr>
-bash: apt: command not found
< luke-jr>
XD
< luke-jr>
aptitude seems to work the same
< wumpus>
that's another question, whether aptitude is affected
< gkrizek>
wumpus: I finally finished my GitHub IRC Service replacement. https://github.com/gkrizek/ghi I had several others ask for it too so I made it really configurable, much like the original service.
< gkrizek>
I'm more than happy to host the service myself, but can help you set it up elsewhere if you prefer.
< wumpus>
gkrizek: awesome!
< wumpus>
looks very neat
< gkrizek>
Thanks!
< wumpus>
and the documentation seems quite clear, I'll probably succeed in setting it up :)
< gkrizek>
wumpus: great, that was the goal! Don't hesitate to ask. I can give you an example '.ghi.yml' file to use if you would like as well.
< gkrizek>
I was thinking it might be nice to add it as a webhook now and have it post to the #bitcoin-commits channel. That way we can test it out and see how it compares to the GitHub Service before it's EOL.
< wumpus>
gkrizek: I've removed the github service from #bitcoin-commits, right setting up an account on my server to run ghi, and sure an example could be helpful :)
< gkrizek>
Awesome, I'll create one
< gkrizek>
wumpus is the bitcoin-git Nick registered (ie; needs a password)?
< wumpus>
gkrizek: it's not registered, probably should be tho
< gkrizek>
Yeah, I would suggest it. But it will work either way
< wumpus>
I use that, only filled in the secret and changed the nick (to not conflict with github), and it says "2019-01-22 22:23:51 [ghi] Found configuration file at '/home/ghi/ghi/.ghi.yml'"
< wumpus>
oh! no, it's picking up the wrong file
< gkrizek>
Where did you put your .ghi.yml that I sent you? The repo contains a template that is in the root of the repo, so you should really edit that. Or delete it and put yours somewhere else
< wumpus>
I've put it in the homedir, but it was picking up the one in the local directory, launching it from somewhere else now
< gkrizek>
Good testing though! haha I didn't quiet think about that being confusing, but I can definitely see where that could cause problems
< gkrizek>
I should probably just delete the template .ghi.yml from the root of the project. That way you are forced to create one on start and it doesn't use the template
< wumpus>
or maybe rename it to ghi.yml.example
< wumpus>
I agree having a life configuration file in the repo is probably not a good idea, wouldn't want to accidentally check it in with the secret in it
< wumpus>
(which is why I used a location outside the repo)
< gkrizek>
Yep, I think you are exactly right.
< gkrizek>
Did the homedir file work now?
< wumpus>
I don't know if I can make it re-send the initial event
< gkrizek>
I'll probably try to iron this out a little better in the future so it installs globally and you can just do `$ ghi start` or something. But didn't get there yet!
< wumpus>
2019-01-22 22:41:39 [ghi] Received the 'ping' event
< wumpus>
2019-01-22 22:41:39 [ghi] Sent 'pong
< wumpus>
that looks better!
< wumpus>
that's what I initially tried, python3 install.py --user, but that made an 'egg' file without a command to launch it :)
< gkrizek>
Ah gotcha. Yeah sorry not there yet! But yes that looks like the correct response!
< wumpus>
okay, now I should merge something I guess
< achow101>
jonasschnelli: should a wallet with disabled private keys be able to import a private key?
< bitcoin-git>
bitcoin/master 94167e2 Wladimir J. van der Laan: Merge #15208: Qt: remove macOS launch-at-startup when compiled with > macOS 10.11, fix memory missmanagement...
< wumpus>
is it supposed to be calling into a shell?
< gkrizek>
Ah, yes. it does to execute the main function. The Server is essentially just an API endpoint that executed the main function. I haven't tested it with /bin/sh, only /bin/bash