< jnewbery> #10882 is squashed and ready for reACK/merge
< gribble> https://github.com/bitcoin/bitcoin/issues/10882 | Keypool topup by jnewbery · Pull Request #10882 · bitcoin/bitcoin · GitHub
< sipa> jnewbery: thanks!
< james0909> hi, i'm having an issue with core, could someone help?
< instagibbs> james0909, #bitcoin please, someone can help there
< praxeology> Would it be worth adding some ECC to chainstate and the block index? Maybe running a memtest before the first synch?
< praxeology> Memtest could auto start on the first synch from genesis... but be skippable
< praxeology> Although... sure memtest is better to be done w/ something like Memtest86+
< gmaxwell> praxeology: error correction wouldn't really help unless it was very very high overhead.
< praxeology> I guess I wasn't really calling for error correction, more error detection. But yea I guess it already does detect an error eventually
< gmaxwell> There is error detection.
< praxeology> Maybe if the client gets stuck on an old block, it should suggest to the user that maybe its database is corrupted... and give some suggestions on what to do about it
< sipa> it does
< sipa> and asks if you want to reindex
< praxeology> ok. james0909 in #bitcoin said his error log said... "ERROR: invalid header received 2017-08-03 01:48:18 ProcessMessages(headers, 11422 bytes) FAILED peer=38"? and his tip was stuck at 478645.
< praxeology> But he didn't say anything about it asking him to reindex... although he did ask if he should reindex.
< gmaxwell> praxeology: why do you think there is corruption...
< gmaxwell> praxeology: the invalid headers messages are bcash nodes getting banned. And he may not catch up if he's been offline until the next block if the peer he picked for the initial header sync was broken or malicious.
< praxeology> he said he tried connecting to other peers.
< gmaxwell> what does that even mean
< praxeology> I guess I didn't confirm that he actually connected to a known good peer
< gmaxwell> doesn't matter f he is 'connected' to a good peer, he won't continue syncing until there is a new block.
< praxeology> oh. I didn't know that
< gmaxwell> (if he was behind and the peer he elected for initial headers was bogus)
< praxeology> So he needs to be connected to a good peer while a new block comes in
< gmaxwell> praxeology: sure, just doing nothing will do that with very high likelyhood since it'll be connected to at least 8 peers.
< praxeology> when bitcoin starts up, it only chooses one peer to ask for latest headers?
< sdaftuar> praxeology: yes
< praxeology> and if that fails... it doesn't ask another?
< gmaxwell> praxeology: right, and if doesn't give all of them it won't learn that it's behind until someone else advertises a block.
< sdaftuar> praxeology: in 0.15, there will be a timeout on that request
< gmaxwell> it can't tell when it 'fails'
< gmaxwell> except for a total timeout, which sdaftuar notes is detected in 0.15
< praxeology> hm, well if the time of his last block is really old... or if the peer he is asking has a different tip...
< gmaxwell> praxeology: different tip than what
< praxeology> if your own node has a different tip than the one you are requesting the latest headers from
< gmaxwell> praxeology: of course its different or fetching headers from it was pointless...
< praxeology> like, their tip is in a different chain or behind or something
< praxeology> bah ok you right
< sdaftuar> you don't request headers from a peer who is on a less-work chain than your tip
< gmaxwell> And any countermeasure has to not do things like terminate a long header sync from a valid peer just because it's a lot of data... otherwise a newly syncing node could get stuck if it's link was too low in bandwidth.
< sdaftuar> actually, i said that wrong. you don't request blocks from such a peer
< gmaxwell> praxeology: there are a bunch of places we could improve responsiveness by punting less helpful peers, but such rules require just a lot of care because false positives are potententially severe attack vectors.
< praxeology> alright... well, its unfortunate I misdiagnosed the guy's issue. Thanks for the help. Hopefully he will come back and ask again on #bitcoin and one of sees him... for his sake... before he -reindex
< gmaxwell> it's also important to not let malicious peers cause users to get any kind of error to the greatest extent that we can avoid it.
< gmaxwell> because unfortunately any warning you show users will cause a small percentage of them to jump off bridges.
< gmaxwell> "I saw a warning that wouldn't go away so I deleted my wallet, and now I still have the warning and where are my bitcoins?"
< mryandao> LOL
< gmaxwell> not really funny though. :(
< gmaxwell> it's not like people are stupid, first they try all the things they think are reasonable, then they try other things...
< gmaxwell> and with enough users, someone is going to think they should try sawing off their hands or whatever.
< Emcy_> throwing your wallet in a lake isnt reasonable
< mryandao> i can't connect the dots between deleting wallet.dat and addressing the problem.
< gmaxwell> mryandao: because it's state that could be causing the error.
< gmaxwell> Emcy_: but they saved the addresses first!
< gmaxwell> you and I know that doesn't help, but it's not exactly obvious
< Emcy_> it was even called "wallet" as a kind of linguistic skeumorph to get people to understand what it is
< mryandao> hmm, this sounds like motivation to decouple key from wallet.dat?
< mryandao> so at least even if wallet.dat was removed out of a panic attack, at least keys is still somewhere safe?
< Emcy_> youre right about explaining pubkey crypto to normies though oh boy
< gmaxwell> well we've talked before about writing wallet backup files periodically but there is a counter argument that doing so will make it more likely for users to get their wallets stolen. e.g. clear wallet off a disk before giving it to someone else...
< Emcy_> i meant to ask why the default datadir on windows in is the domain roaming profile folder, actually, instead of the local profile
< Emcy_> since the subject came up
< praxeology> does bitcoin still use windows registry?
< praxeology> windows configuration/file system usage needs a rework :p
< praxeology> HKEY_CURRENT_USER\Software\Bitcoin\Bitcoin-Qt
< praxeology> Not that I am actually making demands for your time, sorry. Have a good night!
< sipa> praxeology: why?
< goatpig> isn't that just the URI registration?
< praxeology> windows registry is not very compatible with multiple installations in use on the same OS
< praxeology> magic hidden settings that one wouldn't know to move to a new system
< gmaxwell> I wonder if we should adopt a patch to instantly disconnect bcash nodes based on their service flags. Sucks to burn a service bit forever to their recklessness though. :(
< gmaxwell> and presumably the constant inadvertant dos attack of connecting to nodes on another network will move them onto another port eventually.
< praxeology> potentially in the future the burned service code could be recovered in the future after the problem goes away
< praxeology> are they refusing to move to a different port?
< gmaxwell> they refused people people asked them previously.
< gmaxwell> it's not all that easy to recover a service bit where its use results in instantly being disconnected!
< praxeology> Maybe in a month bcash won't have anyone mining it anymore
< gmaxwell> perhaps we shouldn't worry much about burning one because we know we're due for some other pretty substantial p2p revisions, and other updates can add new capabilities flags.
< gmaxwell> I think we only use service flags now for things we absolutely need to have in addr messages, and if we create a addr message replacement (to support NG hidden services and I2P, we'd probably give it a different capabilities signaling tool)
< praxeology> service flags... "flags" implies a 32 bit or 64 bit number?
< praxeology> Maybe switch to a set of service tags instead?
< gmaxwell> well, they need to be small because they're rumored everwhere in addr messages.
< gmaxwell> making them fat sidechannels would likely have clowns using them for file trading. :)
< praxeology> ok, well you could put a size constraint on the tag set... but whatever I'm just sleep dep and over engineering something I don't know enough about
< Eliel> jonasschnelli: is bitcoind's code that does that too difficulty to understand?
< Eliel> (never mind, was looking at the past)
< jonasschnelli> Eliel: depends on you experience
< Eliel> ah true
< gmaxwell> Hi all, karelb is working on the trezor wallet, and they've been trying to use these patches to bitcoin that implement the bitcore address indexing stuff, but they're finding it really slow to the point where performance is problematic.
< karelb> we are using it for a while now :)
< karelb> but right now we are reindexing bitcoin blockchain and it takes foreved
< gmaxwell> Maybe someone would be interested in giving them a bit of a hand at looking at it? (I need to get to bed); I've already provided the standard disclaimers about the inherent non-scalability of address-indexes-of-all-history. :)
< karelb> (It is actually bitcoin-abc, I hope I won't be banned :D, but this issue popped up in bitcoin core too)
< gmaxwell> karelb: one question would be what kind of system is this running on? e.g. is it on some VPS with remote storage that may have poor IO speed?
< gmaxwell> karelb: we'll forgive you for your sins. though obviously can't help with any abc specific issues.
< karelb> it is our local server that has SSD, lots of RAM and processors
< gmaxwell> darn.
< karelb> actually one issue was sort-of ABC related, but it was because of a commit ported from master from bitcoin core, so it will be relevant anyway
< gmaxwell> so since I doubt that addrindex has a useful caching layer, you could look for the leveldb::Options object for the database it creates and try increasing the options.block_cache and options.write_buffer_size to really large levells.
< karelb> it's because the bitcore patches do address index in ConnectBlock/DisconnectBlock, even on the start during the testing thing
< karelb> (jpochyla is my colleague)
< gmaxwell> karelb: probably code based on Bitcoin Core master (including ABC) will not be reliably compatible with that address indexing stuff until it is changed.
< karelb> And the introduction of ApplyBlockUndo somehow caused that
< karelb> yeah. We want to write our own indexing thing that you won't need to put inside the C++ code, since that is a little insane and we need to keep on rebasing that
< karelb> gmaxwell: just fyi, ABC is not based on master, but on 0.14.1, but they took that thing from master
< gmaxwell> karelb: bitcoin core master changed the atomiticity requirements for the backend database, but a side effect of this is that it needs special replay logic to handle crash recovery. ABC has partially ported some of these changes. I am not sure, but I wouldn't be surprised if the address indexing would get corrupted until updated to have the right synchronization behavior.
< gmaxwell> I know it's based on 0.14.1, but they copied some of these database changes.
< karelb> ok
< gmaxwell> in any case, beyond the cache options I mentioned above, I am out of ideas for making it faster without substantial design changes.
< gmaxwell> I think you should probably also make an extra effort to always cleanly start and stop that node, because I wouldn't be confident that it is durable across crashes without corruption.
< Austindoggie> Did it take a long time to reindex because you went back a version of bitcoin core?
< Austindoggie> Sorry if im not allowed to talk here...
< karelb> gmaxwell: hah, that is not a good news. We also noticed the node randomly crashes once in a while and we are not sure why
< gmaxwell> ::sigh::
< karelb> yeah
< karelb> block_cache and write_buffer_size can be set via conf, or only in code?
< karelb> wait I will have a look
< gmaxwell> karelb: run more nodes, reindex anytime one crashes? At least until you can test if the address index is accurate across crashes? I'm not completely confident that it won't be, but it would take some careful review and testing to be sure (especially in the context of ABC that has really scrambled things up a lot)
< gmaxwell> karelb: I think only in the code, I don't think there is an external way to override.
< gmaxwell> in any case, I have to go to bed. Hopefully someone else will wake up and have some other suggestions.
< karelb> (I really hate how ABC reformatted everything for no good reason, but that is another issue from this)
< gmaxwell> yes, it makes it really hard to see what exactly has changed because the reformats were heavily intermixed with real changes. :(
< karelb> @austindoggie nope we started to download blockchain from 0. And it is always stuck on IO for insanely long times
< karelb> and normal bitcoin without bitcore patches doesn't do it, on the same HW
< gmaxwell> karelb: gross final suggestion before I really go: if you really have a lot of ram, create a tmpfs mount big enough for the entire datadir, and sync in there, when it finishes copy it to disk.
< gmaxwell> karelb: the challenge there is that we have insanely optimized bitcoin core's sync process.. we avoid writing to leveldb at all costs, basically, and a significant fraction of UTXO never hit the database at all during normal sync because they're spent before the dbcache fills)
< karelb> thanks a lot for your help
< karelb> going to IRC was a wild shot but it seems it might help :)
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/659c09613408...2e857bb619f5
< bitcoin-git> bitcoin/master 49d903e Alex Morcos: Eliminate fee overpaying edge case when subtracting fee from recipients
< bitcoin-git> bitcoin/master 2e857bb Wladimir J. van der Laan: Merge #10942: Eliminate fee overpaying edge case when subtracting fee from recipients...
< bitcoin-git> [bitcoin] laanwj closed pull request #10942: Eliminate fee overpaying edge case when subtracting fee from recipients (master...subtractfee) https://github.com/bitcoin/bitcoin/pull/10942
< karelb> Hm. options.block_cache and options.write_buffer_size are derived from dbcache
< karelb> dbcache option
< jonasschnelli> Updated to Debian 9 (stretch) and suddenly get gitian build errors: init.lxc: failed to mount /dev/shm : No such file or directory
< jonasschnelli> Anyone else experiences this?
< karelb> maximum dbcache is 16384 MB hard-capped in code; is there a reason for that?
< jonasschnelli> karelb: why would you need more?
< karelb> see the past discussion... we are running bitcore address index patches and they are very slow and stuck on disk operations. And it happens probably because it gets stuck on adding things to address index and commiting it to hard disk on every block
< karelb> probably
< karelb> it must be because of the address index somehow, because on the same PC, bitcoin core without bitcore (sigh) patches runs fine
< karelb> I am actually talking about ABC, but the same issue crops up in bitcoin, plus it might be because ABC added some db logic from master
< karelb> hm, we stopped the bitcoind and it got into some crashed state which is inconsistent and nothing happens
< karelb> :/
< karelb> this is hell
< gmaxwell> karelb: the leveldb caching isn't very useful for the leveldb databases in core because we've already cached the heck out of those things at a higher level... but for the options set for your custom address index it may be very helpful.
< karelb> well it started chugging again, but is still gets stuck. With dbcache set at 16384. Memory has 1GB full and 63GB free
< gmaxwell> karelb: what does stuck mean exactly
< karelb> Log says nothing, and iotop shows 100% and doing a lot of reading/writing in the leveldb
< karelb> and after about 20 minutes, log messages start to appear again
< gmaxwell> dear lord. :(
< karelb> and the binary doesn't reply even to kill signals
< karelb> data/blocks/index is compacting like crazy when it is stuck
< karelb> The bitcoind is stuck and at the same time when this is hapenning
< karelb> and it keeps writing compacting
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/2e857bb619f5...e222618a32a1
< bitcoin-git> bitcoin/master 3498a8d Cory Fields: depends: fix fontconfig with newer glibc...
< bitcoin-git> bitcoin/master e222618 Wladimir J. van der Laan: Merge #10851: depends: fix fontconfig with newer glibc...
< bitcoin-git> [bitcoin] laanwj closed pull request #10851: depends: fix fontconfig with newer glibc (master...fontconfig-bump) https://github.com/bitcoin/bitcoin/pull/10851
< bitcoin-git> [bitcoin] NicolasDorier opened pull request #10980: [WIP] Decouple CKeyStore from CWatchOnlyStore (master...decouplewatchonly) https://github.com/bitcoin/bitcoin/pull/10980
< Mirobit> Doesn't anyone know BCash nodes aren't sending BCash transactions to Core peers? My node has only recieved 2 BCash tx and banned both peers. But the other ~5-10 peers don't seem to send any txs. Weird?
< BlueMatt> achow101: re: #10952: Do you have any idea *how* these folks' wallets got corrupted like that?
< gribble> https://github.com/bitcoin/bitcoin/issues/10952 | [wallet] Remove vchDefaultKey and have better first run detection by achow101 · Pull Request #10952 · bitcoin/bitcoin · GitHub
< BlueMatt> I'm highly skeptical that "just write a new default key" is the right solution here
< BlueMatt> their wallet got confused somehow, auto-fixing without telling them something may be wrong is probably not a good idea
< achow101> BlueMatt: absolutely no idea how they got corrupted
< achow101> I was not able to get a copy of their wallet files either so I couldn't examine them
< achow101> BlueMatt: the solution isn't to "just write a new default key". The solution is to change the first run checker from "there's a valid default key" to "there are keys in the wallet"
< achow101> so with 10952, those wallets would not be considered to be new first run wallets as they have keys but not a valid default key
< wumpus> default key should completely go (for post-0.15 though)
< BlueMatt> achow101: well my point is more broadly that their wallets clearly got corrupted
< wumpus> the check for a new allet should be replaced by a proper "is this an empty database" check
< BlueMatt> achow101: so silently continuing isnt really the right solution
< wumpus> a wallet without any keys yet could in principle be valid
< wumpus> though a bit strange as we initially generate a mempool
< wumpus> (and we don't allow mempoolsize=0)
< karelb> ok we got abc node working, but it was some crazy solution of switching between various binaries and invalidating nodes; it did not crash (yet). But it seems that bitcore patches inside connectblock/disconnectblock are really not the way for the future; it will probably break again (both in abc and later in bitcoin core once you release the db changes)
< achow101> BlueMatt: that would require a specific check for the case that a default key is invalid
< achow101> wumpus: a database is initially created when the wallet file is created. I'm not sure how to check it is empty except that when the database is then read in, there are no keys
< achow101> that's that 10952 does; it checks if there are any keys in the database
< wumpus> yes, that sounds sane
< wumpus> for a new format I'd suggest adding a "database-type" "wallet" k/v pair to distinguish it from other bdb databases, and check for that, but for backwards compatibility that works better
< wumpus> (and no one would be so stupid to put a random other berkeleydb database as wallet.dat? right? :-)
< Lightsword> anyone working on switching bdb out for something else like sqlite?
< achow101> Lightsword: there's a pr somewhere for that I think
< sipa> meh, overkill
< sipa> all we need is a key-value store that's effectively read into memory entirely anyway
< sipa> i had a patch years ago to switch it to an append-only flat file
< Lightsword> the main advantage to sqlite though is that it has good data integrity protection
< achow101> we could upgrade to bdb whatever latest and change how records are stored there
< wumpus> Lightsword: it's quite easy to swap the database for any k/v store, I have a local branch that uses leveldb
< BlueMatt> achow101: if you have a wallet with keys, and a default key that is invalid or missing, your wallet was clearly either corrupted or generated by something other than bitcoin core
< wumpus> achow101: bdb latest (6.x) has serious license issues
< BlueMatt> achow101: continuing silently in that case is not what we want
< Lightsword> sipa, would there not be some potential future use cases where having sql support be helpful?
< achow101> BlueMatt: if there is corruption in the keys themselves or other data, that will be caught elsewhere
< BlueMatt> achow101: we can also change the "first run" check to ignore vchDefaultKey, which we clearly should, but you're pointing to wallets in the wild that have been corrupted and suggesting we should silently continue
< achow101> BlueMatt: I don't think we should care that much about corruption in the default key as it has no use
< sipa> Lightsword: i don't wee how
< sipa> *see
< BlueMatt> achow101: yes we should!
< achow101> BlueMatt: only one of those wallets would silently continue, and the guy had no problems with older versions of core
< BlueMatt> the user should stop using that computer for a wallet!
< wumpus> sql could be useful for metadata kind of things, but meh, I don't think there's really an advantage to it for us we don't index anything
< achow101> BlueMatt: the other wallet ran into other corruption problems
< BlueMatt> that user should be told to throw out their hard drive and get a better one
< BlueMatt> or a new computer
< wumpus> although the current 'keep everything in memory' is kind of dumb
< BlueMatt> not continue
< Lightsword> yeah, I was thinking maybe metadata/multiple accounts or something along those lines could possibly make use of sql
< wumpus> if the wallet would keep things in the database instead and query them when needed, indexes could be useful
< wumpus> accounts are deprecated, if anything we're simplifying the wallet
< achow101> BlueMatt: sure they should be warned, but a corrupted default key should not be a reason to halt the software entirely as default key is useless
< Lightsword> wumpus, what about multiwallet?
< sipa> Lightsword: that uses multiple wallet files
< achow101> with older versions of core, IIRC the default key would have either been overwritten or ignored and allowed the user to continue
< BlueMatt> achow101: if someone's wallet is corrupted, we should exit with an error....if they wish to then restart with -salvagewallet or equivalent, that is also ok
< BlueMatt> achow101: and that is a bug!
< BlueMatt> if we read something and see that someone's hardware is silently corrupting their wallet, we should exit the same way we do with any other wallet corruption errors
< BlueMatt> silently fixing wallets is not ok
< sipa> wumpus: well if we expect to ever need indexes on the future because we won't keep everything in memory, i'd say that sqlite is a good choice
< wumpus> with older versions of core the lack of the default key record would make it assume it's a new wallet, which could do all kinds of bad things? I don't think that's abetter
< BlueMatt> there are clearly bugs here that achow101 identified (that i think need fixing for 15)
< sipa> wumpus: not sure if it's worthwhile though
< BlueMatt> I'm just not sure that silently correcting /anything/ is every good in wallets
< wumpus> for 0.15 it's too lte imo
< wumpus> we should do rc1 asap
< BlueMatt> wumpus: we've seen it in the wild, and it can be a simple fix
< BlueMatt> :(
< wumpus> not add new stuff
< sipa> have we even identified the bug?
< wumpus> but that's just my opinion (and being harried at all sides to do 0.15 asap)
< BlueMatt> sipa: as far as anyone knows its hardware/bdb silently corrupting things
< sipa> as opposed to "we have ween a wallet with no default key, somehow"
< achow101> sipa: I can replicate the problem, not necessarily the cause
< wumpus> but sure if it's a clearly defined problem, with a clearly defined fix, and it can be reviewed in the next days, we can include it
< sipa> achow101: elaborate
< achow101> sipa: encrypt a wallet, use db_dump to dump it, remove the default key, load with db_load, start core, runtime exception
< sipa> well, ok
< wumpus> that's what I expect if you just remove a record
< wumpus> don't do that.
< sipa> but if you permit random file changes, you can make anything happen
< BlueMatt> wumpus: yes, sorry, my understanding is its that kind of "simple fix" that would be easy to get in in a day or two
< achow101> if you do it unencrypted, it works fine
< wumpus> if you delete a random record from the utxo database you'll also be in for a world of pain
< BlueMatt> the issue is that this encourages people to downgrade wallets
< wumpus> we're not resistant to that kind of corruption, and one special case is not going to change that
< achow101> I suppose then a check for a valid default key can be added and that would just be a corruption warning?
< BlueMatt> just making it a nice error message and making sure -salvagewallet works correctly is the Correct (tm) fix, imo
< wumpus> could just as well have been a key record that disappears
< BlueMatt> achow101: yes
< achow101> fine
< wumpus> salvagewallet should work as long as there is any private key left
< achow101> btw the current corruption warnings are kinda bad
< wumpus> that's a known issue...
< wumpus> there's even a chance of running into asserts on db corruption
< wumpus> would be nice if it could detect corruption at run time and offer the user a chance to repair
< wumpus> though if a wallet gets corrupted that's a good indication you should stop using that computer for bitcoin, now
< wumpus> so I agree with BlueMatt in that regard
< achow101> the main problem I have with that is it encourages people who don't know what they are doing (i.e. don't ask, can't use -salvagewallet, etc.) to downgrade
< sipa> ?
< wumpus> why would they think downgrading would work?
< achow101> wumpus: because it does.
< wumpus> say, your wallet is suddenly corrupted one day
< wumpus> why would you consider downgrading?
< wumpus> only if it happens on upgrade I suppose
< achow101> with this specific corruption problem, downgrading to non-hd wallet software (0.12.0- or -usehd=0) will ignore this problem
< sipa> if the corruption ransomly happens right after uograding, i can see why someone would think that
< sipa> *randomly
< sipa> not ransomly
< wumpus> yes, if it happens right after upgrading, but then it's probably a version issue and not a random corruption in any case, would be extremely unlikely otherwise
< sipa> achow101: update wallet version number?
< BlueMatt> anyway, this is why i think we should give an error message
< achow101> sipa: bleh
< sipa> oh, we can't because the optional stuff uses it
< sipa> we really need wallet features grr
< BlueMatt> the crash is better than continuing silently, but if it makes people downgrade, we should instead tell them whats up
< achow101> IIRC salvagewallet didn't fix their problems either, so that will need to be updated
< BlueMatt> sipa: arent you the one who added wallet versioning?
< BlueMatt> achow101: yes, lets do that :)
< wumpus> wallet versioning is used for *optional* features?
< BlueMatt> oh, no
< achow101> I think it would be fine to write some random default key right? it isn't used for anything
< achow101> wumpus: yes, hd and hd chain split or optional, but they have version numbers
< BlueMatt> achow101: its used for pay-to-IP :p
< BlueMatt> (I think)
< sipa> BlueMatt: yes, and now we can't use it
< sipa> because the version number is used for optional features
< sipa> like HD and split HD
< sipa> those should have been a separate record, rather the version number
< wumpus> would have been better to use a new key to indicate those (as well as bump the version number, but not in itself)
< wumpus> well teh version should be bumped too to make it incompatible, but yeah
< sipa> right
< BlueMatt> Make Bitcoin Great Again: Bring back Pay-2-IP
< sipa> BlueMatt: i tried :(
< wumpus> with .onion addresses or some other way to authenticate an IP at the transport layer (such as ipsec) it even could work securely!
< BlueMatt> wumpus: I'm currently working on tcpcrypt for linux patchset :p
< wumpus> BlueMatt: I have no idea what that is
< BlueMatt> wumpus: tcp-crypt :p
< sipa> en..crypt...TCP?
< BlueMatt> yes, that
< sipa> ah, nice
< wumpus> "and your network connections will continue to work even if the remote end does not support Tcpcrypt, in which case connections will gracefully fall back to standard clear-text TCP" - that screams downgrade attack :)
< achow101> oh damn, now default key can't be removed :(
< sipa> wumpus: read on
< sipa> achow101: ?
< sipa> wumpus: it only protects against passive attacks
< achow101> sipa: to check if default key is valid, it needs to be in CWallet
< sipa> achow101: ?
< BlueMatt> wumpus: yes, if the application layer does nto support it it only protects against passive mitm attacks, if the application layer supports it you can disconnect if tcpcrypt failed (but dont do that, cause middleware sucks)
< achow101> I need some way to get default key out of database into a checker, which also checks if default key exists
< sipa> achow101: you can just check when loading the wallet
< achow101> the only vehicle for that right now is cwallet I think
< sipa> i'm sure there is or can be a LoadDefaultKey function on CWallet, called from walletdb
< sipa> switch that to do a check, but not store
< achow101> I guess I could also just add more parameters too
< earlz> I'm trying to get gitian working with 0.14.0. Has anyone seen an error like this? lxcbr0: ERROR while getting interface flags: No such device
< earlz> SIOCSIFADDR: No such device
< earlz> just following the gitian-building.md document exactly and getting that error
< earlz> ifconfig
< earlz> er, woops
< earlz> do I need a kernel flag or something? It's failing at the sudo ifconfig lxcbr0 up 10.0.2.2 bit
< sipa> do you have the lxc kernel module loaded?
< earlz> I pretty much just followed the gitian build instructions and I don't see anywhere in them that the kernel module was explicitly installed unless the lxc package does that
< wumpus> looks like you don't have a lxcbr0 interface for bridging between the lxc and host
< wumpus> not sure how and where that gets created though
< earlz> yea, I'm not understanding where that gets created in the guide. Last time I setup gitian it was for Bitcoin 0.9 so not sure what all has changed since then
< earlz> LXC seems to be broken in some subtle way every time I try to do a new setup of gitian
< sipa> wumpus, BlueMatt: i think we really need to fix the versioning issues
< BlueMatt> wallet versioning?
< sipa> that would make it so much easier (you could say, past wallet version X, no default key is needed anymore)
< sipa> i want to move hd to a separate record
< sipa> rather than store it in the version number
< * BlueMatt> is actually confused why
< sipa> because we can't upgrade the wallet format, ever, now
< sipa> in a backward incompatible way
< BlueMatt> version number = backwards-incompatible change (ie hd), new-key is backwards-compatible extra features
< sipa> without turning the wallet into an hd wallet
< sipa> yes, that's what it's supposed to be
< sipa> but HD is a version number
< BlueMatt> so?
< sipa> we can't increment the version number to indicate something incompatible now
< BlueMatt> what do you want to add that needs to upgrade without being hd?
< sipa> anything
< BlueMatt> we should implement hd upgrade
< BlueMatt> thats something we very much need to do anyway
< sipa> what does that mean?
< BlueMatt> taking a non-hd wallet and adding an hd key
< sipa> yes, ok, that too
< sipa> but do you want to force everyone to have an hd wallet?
< BlueMatt> (probably also need an hd-key-rotate option, but thats separate and I think not related to hd)
< BlueMatt> ^
< BlueMatt> I'm happy to force everyone into an hd wallet if we have an hd-master-key-rotate option
< sipa> yeah, ok...
< sipa> but those are all bigger features
< BlueMatt> well i think all of those are relatively limited code changes
< BlueMatt> and at least hd-master-key-rotate can happen with no format change, I think
< sipa> i am talking about 0.15
< BlueMatt> even if "big" features
< BlueMatt> do we need to add anything else? why rush the no vchdefaultkey thing?
< sipa> otherwise we'll complicate things further for ourselves
< sipa> to add a compatibility layer for split hd
< BlueMatt> ok, I'm missing something...need context
< sipa> no, ignore the vchdefaultkey thing
< sipa> hd and hdsplit, i think, are optional features - things that people can choose not to use
< BlueMatt> now, yes, but i have no problem with them not being optional in the future
< sipa> as they break existing wallet's expectation
< sipa> hmm, ok
< BlueMatt> not being optional in our traditional -walletupgrade sense, that is
< BlueMatt> i mean I'm happy for someone to disagree, I just dont see much downside to it (as long as we have an hd-master-key-rotate option and good documentation on it)
< sipa> i think i agree
< sipa> except i'm not sure we'll have that feature implemented by the time we need it
< BlueMatt> well hd-master-key-rotate is ~trivial with today's format
< sipa> okay
< BlueMatt> hd-upgrade may be slightly less so, I havent thought about it
< BlueMatt> but I expect it to be
< sipa> in that case, i guess it makes sense that those things are in fact in the version number
< sipa> i just wasn't seeing hd as a 'next version', and more as an optional but recommended feature
< BlueMatt> i mean its possible i did /because/ of our versioning scheme, but it is simpler to see it as such and there seem to be relatively limited downsides for it, given some code that doesnt exist yet :p
< wumpus> I'd say using hdsplit isn't really optional, given that you're using hd
< wumpus> hdsplit is a pure improvement on hd
< sipa> agree
< sipa> but do we support upgrading from hd to hdsplit?
< sipa> (right now)
< gmaxwell> ugh. how could we except via invalidating backups which people wouldn't expect...
< wumpus> no, we don't support that right now
< wumpus> hdsplit was sort-of rushed to make 0.15, so that at least new wallets would use it
< sipa> so, until we find a way to (forcefully) upgrade non-hd and hd to hdsplit, we should consider hdsplit an optional feature
< wumpus> but it's strictly superior to hd without hdsplit, so there should be no way to choose that for new wallets
< gmaxwell> I don't think forcefully upgrading is at all possible, because it will invalidate backups.
< wumpus> it wouldn't need to be 'forceful' upgrading, just a *way*
< wumpus> and in any case we don't ever automatically upgrade wallets
< wumpus> (because of backwards compatibility)
< gmaxwell> switch to using segwit, and that will upgrade you. :)
< wumpus> #startmeeting
< lightningbot> Meeting started Thu Aug 3 19:00:54 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< achow101> hi
< jonasschnelli> hi
< sipa> hi
< wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101
< kanzure> hi.
< jtimon> hi
< cfields> hi
< wumpus> 0.15.0rc1 is planned for the 6th (next sunday), so let's go over the open issues again
< wumpus> there's not much anymore
< paveljanik> Hi
< wumpus> (and the scripted-diffs are option, and should be done at the last minute to not conflict with anything else)
< wumpus> Keypool topup #10882 is the most complicated PR open still, but should be almost ready
< gribble> https://github.com/bitcoin/bitcoin/issues/10882 | Keypool topup by jnewbery · Pull Request #10882 · bitcoin/bitcoin · GitHub
< BlueMatt> could go
< BlueMatt> (makes test_bitcoin valgrind-better)
< BlueMatt> and is trivial
< jnewbery> yeah, I've been working on 10882 today. Should be able to push my commits in the next hour or two
< wumpus> I'm not really fishing for new things to add to 0.15
< gmaxwell> jnewbery made a suggestion to fix my outstanding concern in review.
< wumpus> but if there are things that could be merged without affecting anything else that's ok
< jtimon> after #10758, #10919 seems simple to review, it's only +14-13
< gribble> https://github.com/bitcoin/bitcoin/issues/10758 | Fix some chainstate-init-order bugs. by TheBlueMatt · Pull Request #10758 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/10919 | Fix more init bugs. by TheBlueMatt · Pull Request #10919 · bitcoin/bitcoin · GitHub
< sipa> it's also already marked 0.15
< cfields> i think there's a one-liner that could be used to fix the issue in 10977, if it's deemed too much of a change
< gmaxwell> BlueMatt: I'd like to see 10977 fixed! but darn I wish that patch was smaller and easier to review.
< wumpus> yes, just needs some ACKs
< BlueMatt> could be smaller, but is easy to review imo
< sdaftuar> there's one commit in #10919 that i'm hoping others can review/ack
< gribble> https://github.com/bitcoin/bitcoin/issues/10919 | Fix more init bugs. by TheBlueMatt · Pull Request #10919 · bitcoin/bitcoin · GitHub
< wumpus> which one?
< BlueMatt> the first, i believe
< sdaftuar> yep
< wumpus> the threadgroup one? seems obviously sane to me, though it does mean things need to be interrupted too
< BlueMatt> well i think the point is that there is a comment there that notes we dont do it "because dragons"
< BlueMatt> i believe strongly that it is safe, and qt does it, but "dragons"
< wumpus> it is very bad practice not to wait for threads before exiting
< wumpus> yes, qt does it already, it's somewhat less scared of dragons it seems :)
< BlueMatt> isnt qt's logo a dragon or something?
< cfields> heh, think you're thinking of kde
< BlueMatt> oh, yea
< wumpus> (e.g. due to qt's handling of shutdown we also needed #10832)
< gribble> https://github.com/bitcoin/bitcoin/issues/10832 | init: Factor out AppInitLockDataDirectory and fix startup core dump issue by laanwj · Pull Request #10832 · bitcoin/bitcoin · GitHub
< wumpus> anyhow that commit looks good to me, I don't think there's any dragons left
< sdaftuar> sounds-good-to-me-ack
< wumpus> ok, wow, that seems to be all that is left between here and 0.15.0rc1
< BlueMatt> !
< cfields> :)
< morcos> wumpus: i had an assert crash this morning, i imagine it'll be a simple bug.. hopefully i'll have a PR this afternoon, just haven't had time to look at it yet
< wumpus> (there's another PR #10971 by cfields for fixing depends builds, but I don't think that needs disussion, it's a one-liner in the build system)
< gribble> https://github.com/bitcoin/bitcoin/issues/10971 | build: fix missing warnings and sse42 in depends builds by theuni · Pull Request #10971 · bitcoin/bitcoin · GitHub
< wumpus> morcos: ouch, can you open an issue to track it?
< cfields> yea, nothing major
< morcos> yes will open one or the other shortly
< wumpus> ok, thanks
< wumpus> do we need any updates to bips.md for 0.15?
< sipa> hmm, good question
< wumpus> (that's the item in the release process that still has a ? here)
< BlueMatt> is there a bip that recommends hd split?
< sipa> BlueMatt: bip32? :p
< wumpus> there's also "Update `src/chainparams.cpp` chainTxData with statistics about the transaction count and rate." left
< sipa> want me to do that?
< wumpus> and the BLOCK_CHAIN_SIZE, but that's straightforward
< wumpus> yes, if you know what's exactly to be done there that'd help :)
< sipa> sure
< wumpus> thanks
< sipa> short topic: bip173 unit tests issue
< wumpus> #topic bip173 unit tests issue
< jnewbery> There are a few more things for release notes
< sipa> so, bip173 specifies how to translate address strings to witness version/program, and defers to bip141 for encoding that to scriptPubKeys
< sipa> however, the unit tests actually test the whole step from address to scriptPubKey
< sipa> turns out, incorrectly
< sipa> the tests and reference implementation (of the tests) was wrong, and every reimplementation copied it
< gmaxwell> The the error was that it confused hex and dec values.
< sipa> i've made a PR to update the BIP, and all reference implementations i could find, but this is kind of scary
< cfields> corner-cases wrong? or in all cases?
< wumpus> jnewbery: agreed, but release notes need to be finished before -final, not -rc1, so it's not a blocker
< sipa> cfields: it assumed OP_n was encoded as 0x80 + n, rather than 80 + n
< BlueMatt> sipa: so they generate garbage scripts?
< jnewbery> got it. Thanks wumpus
< sipa> BlueMatt: the tests, yes
< sipa> the code itself doesn't contain a conversion to scriptPubKey at all, only a conversion to witness version/program
< gmaxwell> cfields: It was wrong for witness program versions other than 0
< cfields> yikes
< wumpus> oops
< gmaxwell> so this could happily have been deployed and started causing problems when v1 was used.
< sipa> but the tests contain a version/program -> scriptPubKey converter in order to be able the test
< BlueMatt> well if it generated garbage scripts, not much that can be done but fix it
< BlueMatt> are you asking if we should like change the prefix now?
< sipa> no
< sipa> just raising awareness
< BlueMatt> ok, cool
< sdaftuar> perhaps an email to the -dev list would also be good?
< gmaxwell> Also, it highlighes an implementation footgun, I suggested some warning text in the BIP itself. One protection here is that the particular error in sipas' code would result in non-standard outputs.
< sipa> sdaftuar: yes
< gmaxwell> BlueMatt: I did make a suggestion that we consider changing it to break the checksum, but there doesn't appear to be reason to.
< BlueMatt> ok
< morcos> just to be clear what we are talking about, we're not talking about anything merged into Core, but code referenced from the BIP
< BlueMatt> awareness raised!
< gmaxwell> Especially since if someone had slavishly reimplemented the error in the reference, they'd produce non-standard outputs.
< sipa> morcos: indeed.
< morcos> still, a bit scary
< sipa> (though i'd like to PR it to core soon - apparently last week it was suggested to do that in 0.15.1?)
< gmaxwell> Don't start a debate about the name of the version. :P
< sipa> haha
< sipa> (though i'd like to PR it to core soon - apparently last week it was suggested to do that in some soon next version)
< gmaxwell> It also suggests that BIP173 support's test plan should include testing it with other witness version numbers. :)
< sdaftuar> prs welcome :)
< gmaxwell> sipa: well fix the testing shortfalls I found in the C++ version please. :)
< wumpus> PRs to improve the tests are always welcome anyhow
< sipa> gmaxwell: of course
< sipa> anyway, end topic
< wumpus> ok, other topics?
< gmaxwell> uh
< gmaxwell> yes.
< gmaxwell> So service bits and altcoins.
< wumpus> #topic service bits and altcoins
< BlueMatt> wait are altcoins using serice bits now?
< BlueMatt> oh, right 2x did
< gmaxwell> Bcash is using our port, p2pmagic, etc. They distinguish themselve with a blinking service bit.
< BlueMatt> what is wrong with people
< gmaxwell> (also 2x will do this too it seems)
< BlueMatt> gmaxwell: can someone open a pr to change this? or do they refuse to work properly?
< cfields> gmaxwell: i was under the impression that they were planning to change the magic soon
< gmaxwell> We mostly ban them when they send us transactions or headers.
< gmaxwell> cfields: not when I looked three days ago, maybe now.
< karelb> OK, maybe I will ask here. What format are the bitcoin .dat files in data/blocks/*.dat? is that leveldb? what is it exactly?
< jonasschnelli> karelb: meeting, not now
< gmaxwell> If so then the issue becomes moot, otherwise I was going to suggest we ban these bits on connect. The downside is we lose the bits basically forever.
< sipa> they are p2p network format
< karelb> ok sorry
< sipa> oops, yes, layer
< karelb> sorry going out
< * karelb> apologizes
< BlueMatt> gmaxwell: yes, first should be someone bludgening them to work properly
< BlueMatt> gmaxwell: before we start burning service bits
< gmaxwell> BlueMatt: people have been since before they started. Obviously I'll go monitor but I'm not super confident.
< sdaftuar> gmaxwell: why not just ban for eg the next 3 months or something?
< achow101> BlueMatt: gmaxwell IIRC they will be changing their magic and port
< sdaftuar> if we need to do anything at all
< achow101> dunno about 2x
< gmaxwell> One reason burning service bits may not be so bad is because we are due to replace the addr message for i2p and NG-HS support.
< BlueMatt> achow101: what about 2x?
< gmaxwell> So we could at that point just define a new service flagging mechenism.
< BlueMatt> gmaxwell: yea, does anyone have a spec for that?
< gmaxwell> Not yet.
< BlueMatt> k
< gmaxwell> Well if they're finally going to change it, it becomes moot.. but the same issue arises with 2x.
< wumpus> how would a service bit help here?
< BlueMatt> well someone needs to bludgen the 2x folks to change it, otherwise we start banning for a few months
< wumpus> just ban everyone without NODE_SEGWIT? :p
< gmaxwell> wumpus: we still want to support non-upgraded nodes.
< wumpus> but they won't have any new version bit either
< wumpus> that was my point, not to suggest that seriously :)
< gmaxwell> wumpus: oh no, you misunderstand: ABC and 2x both set randomly generated service bits.
< cfields> gmaxwell: eh?
< BlueMatt> I think sdaftuar's suggestion is reasonable, assuming they refuse to do something sane
< gmaxwell> (which they've helpfully ignored the gigantic comment in the code that tells you to at least inform the list.)
< cfields> oh
< gmaxwell> sdaftuar: I hadn't considered a time limited ban. Good point.
< wumpus> oh you mean banning everything that sets their version bit?
< wumpus> yes, that'd be doable
< BlueMatt> wumpus: yes, with a time limit
< gmaxwell> wumpus: well disconnecting, not banning.
< BlueMatt> nah, ban for 3 months
< gmaxwell> Okay thanks, Time limit makes sense. duh.
< wumpus> temporarily, yes
< morcos> wumpus: opened issue #10981, easy fix, but i'll let someone else do the PR as i'm not in the office for next week. please tag with 0.15.
< gribble> https://github.com/bitcoin/bitcoin/issues/10981 | resendwallettransactions asserts if walletbroadcast=0 · Issue #10981 · bitcoin/bitcoin · GitHub
< gmaxwell> BlueMatt: banning creates problems when you run multiple things on one machine.
< BlueMatt> gmaxwell: meh
< wumpus> morcos: thanks, will tag later (not logged in now)
< BlueMatt> gmaxwell: they refused to do something that wasnt astoundingly broken, if it means their users get fucked, its not really my problem
< wumpus> (or if someone else can do it)
< morcos> BlueMatt: so chaincode ip will be banned. nice.
< BlueMatt> morcos: -connect=altcoin.dns.seed
< BlueMatt> :)
< wumpus> agree that banning goes too far, just not allow connections
< sipa> maye just disconnect?
< wumpus> there's no point in banning everything after that
< gmaxwell> what? no. matt, doing that will ban Bitcoin Core users when someone on the same IP ran crapware.
< jtimon> perhaps better for after the meeting, but I'm still not sure why #8498 wasn't suitable for 0.15 ...
< gribble> https://github.com/bitcoin/bitcoin/issues/8498 | Near-Bugfix: Optimization: Minimize the number of times it is checked that no money... by jtimon · Pull Request #8498 · bitcoin/bitcoin · GitHub
< gmaxwell> To be clear this is important because these useless altcoin nodes waste connection slots, and are potentially at risk of gobbling up our initial headers fetch.
< BlueMatt> gmaxwell: ugh
< BlueMatt> fine, disconnect
< BlueMatt> at some point someone is gonna create some altcoin crapware that reconnects agressively, though
< wumpus> disconnect up until a certain date
< BlueMatt> some spv forks probably will
< wumpus> what would be that point?
< BlueMatt> because crapware
< wumpus> it would disconnect immediately after the version message
< gmaxwell> Also, looks like ABC has some kind of deadlocking bug, because I see a few of them just going unresponsive to anything but pings, which delays them getting banned for being on the wrong network.
< morcos> +1 disconnect up to certain date, but i think it should be 12 mos and not 3
< BlueMatt> do not make assumptions about crapware working in a sane way
< wumpus> banning would ban 1 message sooner
< morcos> no reason we'll need that next service bit right away
< sdaftuar> morcos: sure, that sounds fine
< wumpus> s/ban/disconnect
< BlueMatt> morcos: seems reasonable
< gmaxwell> ack on disconnect based on service bits for 12months or similar.
< wumpus> unless we start adding banned nodes to the local firewall, there's no serious difference between disconnecting on connect or after the version message
< gmaxwell> though in general, one of these clowns is going to squat service buts we're in the process of trying to use. :( I have no suggestion on dealing with that.
< morcos> but i think it'd be worth a quick email/github message to jgarzik to check that they aren't imminently changing their plan
< BlueMatt> lets deal with that when we get there
< gmaxwell> s/buts/bits/
< wumpus> well if they change the magic and port we can stop worrying about the service bits they claim
< BlueMatt> morcos: yes, as I stated previously we should tell these guys to change network magic *first*
< wumpus> also we could at that point check for NODE_SEGWIT + our service bit
< morcos> yes but what if they change to a different service bit
< gmaxwell> morcos: there is some PR where people have been arguing for ages, about this, so I'm doubtful but sure.
< morcos> might as well ask first and tell him what we're planning on doing
< wumpus> change to a different version bit? what would that accomplish?
< morcos> whooo knows?!!
< gmaxwell> At the end we're doing them a favor, there are a lot more bitcoin nodes than random altcoin nodes, so these incorrect connections tend to cause them a lot more problems than us.
< BlueMatt> yup
< BlueMatt> ok, probalem solved
< BlueMatt> who wants to go tell them that we're gonna disconnect them?
< wumpus> if avoiding detection is the point, they could better stop setting their version bti at that point is better than randomly cycling it according to moon phases
< gmaxwell> BlueMatt: perhaps we should just open the disconnect PR and ping them to comment on it?
< wumpus> bleh
< BlueMatt> gmaxwell: wfm, but seems like someone should open an issue
< BlueMatt> I vote morcos does it
< gmaxwell> throw him to the wolves... enh? what did he do so wrong?
< BlueMatt> actually, it was sdaftuar's idea, he can do it
< gmaxwell> throw him to the wolves... enh? what did he do so wrong?
< wumpus> seems we'd rather not invite certain discussions to our github but eh
< BlueMatt> gmaxwell: fine, I'll deal with it
< gmaxwell> BlueMatt: good, I know what you did so wrong. :P
< jtimon> but if the bits are selected randomly, how does burning them help?
< sipa> jtimon: s/randomly/arbitrarily/
< wumpus> they aren't selected randomly, they're not doing service bit hopping or something like that
< sipa> they're not different every time
< sipa> they just arbitrarily picked one
< jtimon> sipa: I see, thanks
< gmaxwell> jtimon: I don't follow your question. The altcoin efforts have selected randomly but hardcoded the result or their fair dice roll. :)
< morcos> +1 sdaftuar doing it... i'm trying to pack
< jtimon> gmaxwell: yeah, got it
< gmaxwell> and just failed to follow the giant comment in the code to make a public announcement about it even.
< wumpus> e.g. they have monkeys throw darts to select one when they need it, not every connection
< morcos> oh nm, or bluematt
< BlueMatt> I think morcos is clearly just trying to throw anyone else under the bus, sounds like he should do it, then :p
< BlueMatt> anyway, next topic?
< gmaxwell> ;;action bluematt goes under the bus
< * gribble> bluematt goes under the bus
< gmaxwell> see, the robot overlords agree
< BlueMatt> ;;action goes under the bus
< * gribble> goes under the bus
< achow101> we can't/shouldn't ban 2x peers until they fork
< BlueMatt> achow101: yes we should
< wumpus> I think this was mainly about BCC which already forked
< achow101> BlueMatt: why? we won't be giving them invalid stuff until they fork, and vice versa
< gmaxwell> hash that out outside of the meeting plz.
< wumpus> achow101: on the other hand, adding logic to the code to check whether they've forked would complicate things more than just disconnecting on a service bit
< wumpus> yeah
< BlueMatt> next topic?
< gmaxwell> But in general if someone is going to make broken software, we can only go so far to accomidate it.
< wumpus> we've run out of topics
< achow101> wumpus: we know when they activate. at block X start banning them
< gmaxwell> I doubt we do.
< jtimon> perhaps better for after the meeting, but I'm still not sure why #8498 wasn't suitable for 0.15 ...
< BlueMatt> achow101: I'm not jumping through hoops to make sure altcoins stay in consensus until *right before* they fork...
< gribble> https://github.com/bitcoin/bitcoin/issues/8498 | Near-Bugfix: Optimization: Minimize the number of times it is checked that no money... by jtimon · Pull Request #8498 · bitcoin/bitcoin · GitHub
< wumpus> #endmeeting
< lightningbot> Meeting ended Thu Aug 3 19:48:33 2017 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< gmaxwell> since they still don't have a correct published specification and keep changing things.
< achow101> gmaxwell: they have stayed on using 12960 blocks after segwit activation
< gmaxwell> achow101: and what happens if they pull it up to 0 blocks, with a weeks notice? They're clearly being adversarial and trying to harm things, otherwise they'd change port/magic.
< BlueMatt> achow101: and what if they change it next week? No point, if they want to make sure they stay in consensus, they can run bridge nodes, its not hard
< achow101> we risk partitioning the network since there are miners that use btc1
< BlueMatt> achow101: that can be remedied otherwise
< BlueMatt> (and will be)
< BlueMatt> and, mostly, hopefully, they just change network magic
< BlueMatt> and then they can do whatever they want
< bitcoin-git> [bitcoin] TheBlueMatt opened pull request #10982: Disconnect network service bits 6 and 8 until Aug 1, 2018 (master...2017-08-bad-service-bits) https://github.com/bitcoin/bitcoin/pull/10982
< jnewbery> gmaxwell BlueMatt new commits in #10882 ready for review
< gribble> https://github.com/bitcoin/bitcoin/issues/10882 | Keypool topup by jnewbery · Pull Request #10882 · bitcoin/bitcoin · GitHub
< BlueMatt> thanks
< Cobra-Bitcoin> Any core dev around?
< sipa> many
< Cobra-Bitcoin> Anyone know what btcdrak means when he says "future releases will be linked" here https://github.com/bitcoin-core/bitcoincore.org/issues/33?
< Cobra-Bitcoin> So the release notes will point users to bitcoincore.org binaries?
< sipa> BlueMatt: ^
< BlueMatt> Cobra-Bitcoin: that is the intention, yes, I mailed you about this a while back :). I think the goal is to start pointing people to bitcoincore.org, but also sicne y'all wanted to keep mirroring on bitcoin.org (and cause no point telling people to go somewhere else if they're already used to bitcoin.org), they'd be in both places
< BlueMatt> ie cause security isnt helped by making people make hops
< BlueMatt> Cobra-Bitcoin: see-also #10651
< gribble> https://github.com/bitcoin/bitcoin/issues/10651 | Verify binaries from bitcoincore.org and bitcoin.org by TheBlueMatt · Pull Request #10651 · bitcoin/bitcoin · GitHub
< Cobra-Bitcoin> But is bitcoin.org not doing enough to already distribute the binaries effectively?
< Cobra-Bitcoin> We have no choice but to mirror, since we have a lot of pages structured around the binaries and Core
< achow101> wumpus: BlueMatt so what do we do about salvagewallet and this default key thing?
< achow101> write a new one?
< BlueMatt> Cobra-Bitcoin: see pm
< BlueMatt> achow101: I havent looked at salvage, I mean certainly making it return an error instead of throwing like it does is not too hard
< wumpus> Cobra-Bitcoin: hosting on both bitcoin.org and bitconcore.org is perfectly acceptable, preferable even IMO
< achow101> BlueMatt: I have it return a db corrupt error now (local change, not yet pushed)
< wumpus> Cobra-Bitcoin: more (trusted) places to get the binaries from gives some redundancy
< BlueMatt> plus what wumpus said
< achow101> but my concern is that people will downgrade because downgrading works. there's no way to prevent downgrade is to use a new wallet version, but that's a mess that I don't want to deal with right now
< BlueMatt> achow101: well a reasonable error message should help, then
< BlueMatt> I dont care about preventing users from doing X or Y, but giving them an error message informing them that their hardware appears to have silently corrupted their wallet is appropriate
< BlueMatt> ideally salvagewallet would handle this case (does it not, my understanding was it should?)
< achow101> currently it says "Error loading wallet.dat: Wallet corrupted" with a more specific "invalid default key" message in the debug.log
< BlueMatt> thats fine?
< achow101> salvagewallet doesn't handle uncorrupting keys, it only pulls data that it can find being valid and passes it through
< achow101> so any corrupted keys will get passed through to a salvaged wallet
< achow101> (not like you could uncorrupt a key anyways)
< wumpus> is there even such a thing as 'uncorrupting keys'?
< wumpus> it's not like we encode in some redundant way that allows recovery
< wumpus> we could do that, but we don't
< achow101> wumpus: well you could easily uncorrupt a default key by just writing a generic valid key like the generator
< wumpus> you could erase/ignore corrupted keys, though that's somewhat scary...
< BlueMatt> wumpus: isnt salvagewallet always scary?
< wumpus> (but not much different from what bdb salvage already does)
< wumpus> yes, sure, but this isn't black and white
< BlueMatt> true
< wumpus> there are certainly ways to make it even scarier :)
< sipa> so there is an added point here: any key that got corrupted ever probably resulted in the wallet just failing, and the user starting over or finding other solution
< sipa> but if it was the default key that was corrupted, it likely just meant that the wallet replenished the keypool at every startup, ever
< sipa> and everything kept working
< wumpus> yes
< sipa> so there may be a survivalship bias, resulting in currently a higher than expected number of people with a corrupted default key
< wumpus> there probably should be a way to salvage and ignore corrupt keys...
< sipa> yes.
< wumpus> (another salvage level!)
< sipa> unfortunately, not possible for encrypted keys
< sipa> at least not without passphrase
< wumpus> yeah...
< wumpus> repair for an encrypted wallet should ideally ask for the passphrase
< wumpus> it's the only way to recover most effectively
< achow101> it also seems like the two reported cases of this involve wallets that have corrupted keys, which salvagewallet doesn't help with
< achow101> it's just that the user doesn't notice those keys were corrupted until they try to spend
< achow101> then the wallet fails to decrypt
< wumpus> yes that's not good
< achow101> so i guess we could just leave this as a warning about wallet corruption and not do any salvaging which would exacerbate the problem?
< wumpus> agreed, any 'salvaging' should be at user initiative
< earlz> Is there any easy way to make gitian skip building dependencies? I'm having some trouble with a modification I made breaking the gitian build for bitcoind itself, but the 2 hours spent compiling dependencies is killing me
< cfields> earlz: the dependencies are cached, they'll only be built once
< earlz> cfields: that does not seem to be the case if gbuild fails
< earlz> is there some command line argument or something I'm missing to prevent it from saving the cache, cause right now I basiclaly do gbuild, it compiles all deps, then encounters failure, I'll apply a fix and commit/push it, then update the gbuild command for the new commit hash and I observe it compiles all deps again
< cfields> earlz: do a vanilla build, make sure it finishes, get the deps cached
< cfields> then rebuild with your changes
< earlz> oh I see, so it only caches if the build succeeds?
< cfields> yes
< bitcoin-git> [bitcoin] TheBlueMatt opened pull request #10984: Allow 2 simultaneous (compact-)block downloads (master...2017-08-paralell-block-downloads) https://github.com/bitcoin/bitcoin/pull/10984
< BlueMatt> cfields: when you do the next round of cleanups on #10756, can you not move InitializeNode/FinalizeNode? I'm starting work on building on top and have patches that conflict :/
< gribble> https://github.com/bitcoin/bitcoin/issues/10756 | net processing: swap out signals for an interface class by theuni · Pull Request #10756 · bitcoin/bitcoin · GitHub
< cfields> BlueMatt: heh, i thought we agreed that was the next step?
< BlueMatt> cfields: nono, i mean dont move the code down in the file
< BlueMatt> not dont move
< cfields> BlueMatt: oh, I see what you mean
< cfields> BlueMatt: they have to move out of the anon namespace though :(
< BlueMatt> ohh, didnt realize they were in a namespace
< BlueMatt> damn, ok, fine
< BlueMatt> I'll just have to do some dirty rebase
< cfields> yea, that's the only reason i moved them
< cfields> could do something ugly like closing/reopening the namespace, but i'd rather not :)
< bitcoin-git> [bitcoin] sipa opened pull request #10985: Add undocumented -forcecompactdb to force LevelDB compactions (master...20170803_forcecompactdb) https://github.com/bitcoin/bitcoin/pull/10985
< bitcoin-git> [bitcoin] sipa opened pull request #10986: Update chain transaction statistics (master...20170803_txstats) https://github.com/bitcoin/bitcoin/pull/10986
< gmaxwell> #10985: \0/
< gribble> https://github.com/bitcoin/bitcoin/issues/10985 | Add undocumented -forcecompactdb to force LevelDB compactions by sipa · Pull Request #10985 · bitcoin/bitcoin · GitHub
< sipa> gmaxwell: ^ untested