< bitcoin-git>
[bitcoin] pierreN opened pull request #11525: Utils and libraries: fix warning -Wsign-compare in ConsumeDecimalNumber (master...fix_sign_compare_lvldb_logging) https://github.com/bitcoin/bitcoin/pull/11525
< bitcoin-git>
[bitcoin] fanquake closed pull request #11525: Utils and libraries: fix warning -Wsign-compare in ConsumeDecimalNumber (master...fix_sign_compare_lvldb_logging) https://github.com/bitcoin/bitcoin/pull/11525
< wallet42>
Would it be a lot of change to have an -option so that when spending coins in the chainstate db instead of deleting the key, it will be renamed from c[txid][nout] to s[txid][nout] ? basically have it ever growing coins db?
< wallet42>
it would ease some of my analysis tools
< bitcoin-git>
[bitcoin] sipsorcery opened pull request #11526: Visual Studio build configuration for Bitcoin Core. (master...build_msvc) https://github.com/bitcoin/bitcoin/pull/11526
< bitcoin-git>
[bitcoin] schildbach opened pull request #11527: Remove my testnet DNS seed as I currently don't have the capacity to … (master...remove-seed) https://github.com/bitcoin/bitcoin/pull/11527
< bitcoin-git>
bitcoin/master 132d322 Andreas Schildbach: Remove my testnet DNS seed as I currently don't have the capacity to keep it up to date.
< bitcoin-git>
bitcoin/master 13f53b7 Wladimir J. van der Laan: Merge #11527: Remove my testnet DNS seed as I currently don't have the capacity to …...
< sipa>
promag showed a much smaller change earlier
< wumpus>
ow diffing with w=1 makes it better
< wumpus>
still I don't see why so much logic needs to be changed for this
< sipa>
still, what i saw earlier was 1 line
< sipa>
but inside GetTransaction rather than in the caller
< promag>
see last link
< wumpus>
sounds good to me
< sipa>
^ that
< promag>
done
< promag>
although I think we can only lock cs_main after mempool lookup
< sipa>
you can hold both
< sipa>
but you need to grab main first, if you need both
< promag>
really?
< sipa>
yes
< sipa>
there exists a lock order
< promag>
ah got it, so the other commit was wrong
< sipa>
which means that if there is one place in the code where A is being held while B is being taken, you can't have another place where A is being taken while B is held
< promag>
ok
< sipa>
the opposite is called lock order inversion, and is a risk for deadlocking
< sipa>
-DDEBUG_LOCKORDER automatically detects lock order inversions
< promag>
thanks for the explanation
< wumpus>
is there something wrong with travis again or is everyone just submitting PRs that break it? :)
< Chris_Stewart_5>
Is there numbers any where of how long it takes to validate/verify a block w/ 0.15.0
< wumpus>
-debug=bench
< bitcoin-git>
[bitcoin] laanwj closed pull request #11507: [RPC] Don't do slow transaction lookup when txindex is enabled (master...getrawtx-txindex) https://github.com/bitcoin/bitcoin/pull/11507
< promag>
in travis, what's the point in running functional tests if make check fails?
< cfields>
heh, all 0.15.0.2 issues are pretty closely intertwined.
< cfields>
#topicsuggestion ^^ :)
< wumpus>
promag: the functional tests also don't work like that, if one test fails it continues with the rest. I guess it's useful to see exactly what tests fail.
< wumpus>
if it stops at the first failed test you need to re-run time after time after fixing a single test
< ossifrage>
That copy paste problem I reported earlier is an actual QT bug: "GUI: QXcbClipboard::setMimeData: Cannot set X11 selection owner"
< ossifrage>
It seems to have been triggered by running a second instance of firefox as another user.
< ossifrage>
It is possible that using the QClipboard wrapper instead of the QXcb* stuff will address some of these edge cases...
< wumpus>
yea X has some interesting clipboard issues sometimes, especially combined with 'remote' windows, or with some software that tries to keep up to date with the clipboard and querying it all the time
< ossifrage>
This one was a bit of a usability issue and I ended up having to use bitcoin-cli to copy and paste addresses without screwing up
< wumpus>
I'm sure we don't use QXcb clipboard stuff directly in our code
< wumpus>
it's possible to build bitcoin-qt w/ wayland, for example, and the clipboard works
< ossifrage>
firefox, chrome, xfce-terminal where all un-effected, but gnome-terminal and bitcoin-qt where
< wumpus>
xclip is a useful tool to probe around with X clipboards, did you know it actually has three of them?
< ossifrage>
wumpus, the clipboard worked fine, I use it extensively with bitcoin-qt, until I launched a 'lazy sandboxed' firefox as another user
< ossifrage>
wumpus, yeah I tried to use xclip to clear all 3 hoping that would magically reset whatever permissions it was confused about, with no love
< wumpus>
lol it's such a mess
< ossifrage>
Restarting gnome-terminal fixed the problem for it, but I didn't want to restart bitcoin-qt
< wumpus>
but hey windows has to be better at something :)
< ossifrage>
(I have some sort of uptime/connection count fetish)
< ossifrage>
I used to run X stuff over ssh tunnels for years and never remember having a problem like this...
< wumpus>
I've had more clipboard issues with X than I can even remember
< ossifrage>
Weirdness yes, but complete failure to work, not so much...
< wumpus>
it's most fun VNC or xpra and remote windows get involved, sometimes they get into a loop updating
< cfields>
ossifrage: huh, very interesting. my Konsole copy/paste quit working yesterday.
< wumpus>
or sometimes you suddenly get old clipboard contents back
< ossifrage>
wumpus, yeah I've seen that before and I think clipboard managers make the problem even worse
< wumpus>
and copy some password into something that aboslutely shouldn't have it
< wumpus>
ossifrage: yup like the one that KDE comes with
< ossifrage>
wumpus, I have a shell alias for clearing the clipboard after pasting a password :p
< wumpus>
cfields: interesting, maybe it's some recent update that made it worse
< cfields>
sounds reasonable
< cfields>
I didn't investigate much because i use vim's yank for the most part. I just chalked it up to a Konsole bug.
< wumpus>
well at least the clipboard becomes one of the yank registers, not the default one
< cfields>
dammit, I've squashed/force-pushed this pr like 5 times now, I keep missing little things I forgot to address. I hope that's not blowing up anybody's mailbox?
< wumpus>
oh wait, even two of them, + and *, for two differnt clipboards, of course
< wumpus>
cfields: well not mine at least
< cfields>
k. wasn't sure if you could subscribe to new commits, even if they're just a rebase
< wumpus>
thinking of it, I don't get mails for pushes at all, just for comments
< wumpus>
not that I know of
< cfields>
I got them for pushes at some point. Can't remember if i turned it off for being to spammy, or if Github disabled that
< cfields>
*too spammy
< achow101>
I definitely get emails for new commits
< wumpus>
achow101: but also for PRs you're subscribed to?
< MarcoFalke>
I get for pushes, but not for force pushes
< achow101>
but there's a delay between pushing a commit or making a comment and then getting the email. I think github is smart enough to not send out too many mails if you make multiple pushes within a minute or two of each other
< achow101>
wumpus: this is for PRs
< wumpus>
ok, strange, well I don't really mind not getting them, I already get way too much mail
< cfields>
achow101: ok. for future reference, did you just get about 5 from me for 11512? Or it skipped them all because they were force-pushes?
< cfields>
er, 11457
< achow101>
cfields: I did not get any emails about those force pushes
< cfields>
great, thanks
< ryanofsky>
i think github doesn't send "pushed commits" messages for prs when the new commits have the same subject lines as the old commits
< cfields>
ryanofsky: well about 3 of my force-pushes right now were squashing "temp" and "fixup" commits that I accidentally left in. So it apparently doesn't notify on new ones, at least
< cfields>
or, maybe only after some delay like achow101 said
< wumpus>
it probably only mails when a new commit (or more) was added on top of the last known one
< cfields>
ah, that would make sense
< wumpus>
meeting time?
< jonasschnelli>
jup
< meshcollider>
Hi
< wumpus>
#startmeeting
< lightningbot>
Meeting started Thu Oct 19 19:00:32 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< wumpus>
anything that is close to merging, or that needs further discussion?
< cfields>
BlueMatt: are you around to fill in some of the cb interactions with those PRs? They're hard to review because of subtleties I assume I'm not fully accounting for.
< achow101>
I wasn't able to do as much as I wanted to this week because of exams. I'll fix up #11446 later today hopefully
< BlueMatt>
sdaftuar: (and somewhat I) worked out a replacement for #11487, that I need to rewrite and close the original
< gribble>
https://github.com/bitcoin/bitcoin/issues/11487 | Check that new headers are not a descendant of an invalid block by TheBlueMatt · Pull Request #11487 · bitcoin/bitcoin · GitHub
< BlueMatt>
so I'll do that a bit later
< wumpus>
ok
< sdaftuar>
i updated #11490 with some bug fixes and addressed feedback. i also added a unit test that passes locally but fails in travis :( workin gon it...
< jonasschnelli>
lables / account are completely on the wallet level... yes.
< wumpus>
yes, they're at the wallet level, I agree
< jnewbery>
I don't think 11497 addresses the problem of migrating from an account system to a label system, although I may not have fully understood what the plan is
< achow101>
yeah, I agree with that. 11497 was really just done in a fit of anger after I got too fed up with handling account things
< wumpus>
though from the perspective of the database there's no difference, accounts and labels use the same underlying strucures
< meshcollider>
Agreed but there were concerns bumping the wallet version because of HD wallets right
< sipa>
meshcollider: has nothing to do with accounts
< wumpus>
it doesn't need a new wallet version
< jnewbery>
meshcollider: yes. Is now a good time to raise the topic of feature flags on wallets?
< meshcollider>
Oh I thought that was discussed in 7729
< wumpus>
accounts do not introduce any new wallet features , at least this first incarnation simply does what the GUI does
< achow101>
meshcollider: it's the same database structures, so that shouldn't matter
< meshcollider>
Yeah I thought 7729 comments were discussing using the version number as a flag for the label/account switch
< wumpus>
people are requesting things like allowing multiple labels for one address, which would need extensions to functionality, but the current one is simply what is already done by the GUI
< jonasschnelli>
(another topic, but): we can bump the wallet version because all new wallets now must be HD, right?
< wumpus>
seems unnecessary
< sipa>
i think using -deprecatedrpc is fine for accounts - it's literally just removing some functionality
< wumpus>
sipa: yeah, it removes functionaltiy from the RPC interface
< sipa>
but we may first need to add label-based RPCs
< wumpus>
it's an RPC interface thing
< wumpus>
sipa: yes, 7729 needs to go in first
< sipa>
okay
< wumpus>
then the flag can select between the old account based interface and the new label based one
< wumpus>
at least for 0.16, in 0.17 I suppose the old interface will simply go away
< achow101>
maybe we should use a different flag than -deprecatedrpc
< meshcollider>
So yeah jnewbery topic suggestion of wallet feature flags?
< wumpus>
this won't lose compatiblity with old wallets
< wumpus>
it just will lose access to any account info in them
< sipa>
right, it's not a wallet feature
< wumpus>
so it's not really a wallet versioning thing
< jonasschnelli>
meshcollider: no flags required for the label/account switch (only changes on functional level)
< sipa>
it's a sofrware feature that can be turned on or off
< jnewbery>
ok, so it sounds like it's not necessary for accounts/labels. I'll go back and look at 7729 again.
< * achow101>
is confused now
< wumpus>
#action implement and merge the label api finally
< wumpus>
ok, next topic?
< sipa>
achow101: the big idea is that all accounts functionality remains under the name "label", except the concept of an acount balance
< achow101>
sipa: so it is a node thing and not wallet based?
< sipa>
achow101: wut?
< sipa>
achow101: no, it's an RPC thing
< sipa>
the wallet or node are unaffected
< wumpus>
right
< achow101>
s/node thing/rpc thing/
< sipa>
right
< jonasschnelli>
the wallet structure is unaffected although the wallet code is
< sipa>
is it?
< jonasschnelli>
if we consider rpcwallet as wallet code
< sipa>
i wouldn't expect it to (until it is deleted entirely)
< wumpus>
some small changes are necessary
< sipa>
ok
< sipa>
anyway, details
< sipa>
i'm going off soo
< jonasschnelli>
meshcollider: in case you want to work on wallet flags (there is already a closed PR) #8369
< wumpus>
there's some functionaltiy that would be in the label API but the GUI doesn't use at the moment, from what I remember, but anyhow yeah it's minimal
< gribble>
https://github.com/bitcoin/bitcoin/issues/8369 | [FOR LATER USE][WIP][Wallet] add support for a flexible "set of features" by jonasschnelli · Pull Request #8369 · bitcoin/bitcoin · GitHub
< jnewbery>
Last comment from wumpus in 7729 included "Either label or account RPCs could be usable based on the wallet version." I think we probably need to look at that PR again to refresh our memories (I certainly do)
< meshcollider>
jonasschnelli: Oh right, I'll have a look
< sipa>
okay
< wumpus>
jnewbery: what a dumb comment!
< wumpus>
going to remove it immediately :-)
< jnewbery>
I think it was in response to me, more as a 'this is how it could possibly work'
< sipa>
pffft, fact based analysis, how boring
< meshcollider>
Yeah that comments probably what I was thinking of
< wumpus>
back then I probably thought using the wallet version was a useful way of doing that flagging, but I reconsider
< achow101>
well now we have -deprecatedrpc
< sipa>
right
< wumpus>
yes
< sipa>
no more topics?
< wumpus>
nope
< sipa>
#halfameeting
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Oct 19 19:31:27 2017 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< gribble>
https://github.com/bitcoin/bitcoin/issues/10579 | [RPC] Split signrawtransaction into wallet and non-wallet RPC command by achow101 · Pull Request #10579 · bitcoin/bitcoin · GitHub
< wumpus>
or alternatively, everyone should be reviewing 0.15.0.2 PRs
< achow101>
that too
< wumpus>
we have kind of a strict deadline there
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #11530: Add share/rpcuser to dist. source code archive (master...Mf1710-distShare) https://github.com/bitcoin/bitcoin/pull/11530
< earlz>
In GetTime(), it uses time(NULL)'s result directly. The returned value doesn't appear to be standard guaranteed to be UTC/unix time though. Shouldn't GetTime use gmtime or something similar to correct ofr this?
< bitcoin-git>
[bitcoin] TheBlueMatt opened pull request #11531: Check that new headers are not a descendant of an invalid block (more effeciently) (master...2017-10-cache-invalid-indexes) https://github.com/bitcoin/bitcoin/pull/11531
< meshcollider>
That should be added to 0.15.0.2 milestone ^