< BlueMatt>
why is the hd wallet seed interpreted as an ecdsa privkey in a few places?
< BlueMatt>
that seems like type confusion in a cryptographic system
< BlueMatt>
though afaict its not really used as a pubkey anywhere, this seems like a very, very bad idea generally
< BlueMatt>
(every way I can think about it its nbd, just seems like bad practice)
< sipa>
BlueMatt: it's the other way around... we generate a (valid) private key, and then reinterpret it as a seed
< BlueMatt>
yes, this seems strange
< sipa>
this surprised me too
< BlueMatt>
because it seems to be the pubkey thing is only used to go find the data that we want to hash
< sipa>
but i only noticed it after merge, and i can't find any problems with it
< BlueMatt>
yea, I mean it doesnt seem to be broken, just bad practice
< BlueMatt>
(for encrypted wallets we store the pubkey-version in plaintext on disk, afaict, which is not ideal)
< BlueMatt>
^ about the only real issue I see
< gmaxwell>
more or less bypasses the kdf then.
< sipa>
what kdf?
< sipa>
bip32 does not have any
< gmaxwell>
The computationally hard function used for the passphrase in wallet encryption. But nevermind, I'm incorrect, it doesn't bypass it.
< sipa>
ah, for encryption - no, indeed
< BlueMatt>
gmaxwell: no, remember the padding bug
< BlueMatt>
:(
< gmaxwell>
I don't consider the padding bug interesting.
< sipa>
still needs KDF; just no ec operation anymore
< BlueMatt>
gmaxwell: indeed, I consider it a novelty
< sipa>
BlueMatt: the alternative for the bip32 derivation is that we'd use the private key of the generated root as master key (with chaincode stored on disk), rather than treating it as a seed
< sipa>
which would pretty much have identical security properties
< BlueMatt>
sipa: kinda, ideally we wouldnt store the pubkey of that on disk
< BlueMatt>
sipa: actually, your suggestion is worse
< BlueMatt>
well, different
< gmaxwell>
I believe there is a way to quickly disqualify candidate private keys without EC operations in any case; assuming you know two signatures with that key.
< BlueMatt>
well, maybe better? right now you need to have your wallet unlocked to create new pubkeys (ie scan the chain, ish)
< BlueMatt>
but we could store master pubkey on disk and then we wouldnt
< BlueMatt>
dunno if better, but different
< sipa>
BlueMatt: it uses hardened derivation, so you always need to unlock in any case
< BlueMatt>
gmaxwell: mmm, didnt know that
< gmaxwell>
BlueMatt: that would require using public derrivation.
< BlueMatt>
sipa: ahh
< BlueMatt>
well, I mean you could store that as well
< BlueMatt>
is there any way to get an rpc or so to give you the master key?
< BlueMatt>
if you dump it, probably?
< BlueMatt>
the way I read the current code, every time we go to GenerateNewKey() we will iterate over every key we have already generated before generating a new one (as we never call SetHDChain to store the updated counter on disk)....anyone with a test hd wallet lying around want to see what their disk-counter is?
< BlueMatt>
hmm...I also dont see any discussion of the non-switch of FEATURE_LATEST on the pr....this might complicate future wallet changes...it seems to me that this locks us in such that the next change to wallet version must either be the same non-default or we will, at that time, have to force hd wallets to be the default
< BlueMatt>
I'm not strictly aginst this, just noting that it seems it was not at all discussed but locks us into a future path
< BlueMatt>
oh ffs, ok, I'ma copy this into the hd wallet pr
< GitHub184>
bitcoin/0.13 d6bb231 Wladimir J. van der Laan: Merge #8360: doc: Add a few items to release notes...
< wumpus>
going to tag rc1 in a bit
< wumpus>
anything that really still needs to go in? last chance
< jonasschnelli>
wumpus: what about TheBlueMatts concern about FEATURE_HD?
< jonasschnelli>
I have worked out a PR that can support a set of strings (features).
< wumpus>
I don't see it as a large problem
< jonasschnelli>
But probably to late for 0.13
< sipa>
i think string-based features can wait until later
< wumpus>
over time, HD wallets are probably going to be the default anyway, and the at-least-0.13 requirement is not a big issue
< jonasschnelli>
The only concern is, that if we introduce another feature (lets say KEY_ORIGIN_FLAGS [there is a PR]) that this feature would require 0.13 even if it would be compatible with < 0.13.
< wumpus>
I think how you handled the transfer period is fine
< MarcoFalke>
Jup, we can just make -upgradewallet take care of this
< MarcoFalke>
There is plenty of time till .14
< wumpus>
jonasschnelli: I don't see that as a big problem honestly
< jonasschnelli>
Yes. Agree.
< jonasschnelli>
But let me PR the set-of-strings feature work for later usage
< wumpus>
yes, we need a better feature system for the wallet, but it's not urgent
< sipa>
string-based features is a much more clean solution... but the current one works fine as long as versioning is completely serial
< jonasschnelli>
Yes. The optional HDness made it a bit more complex.
< wumpus>
I don't see the-next-feature-will-require-at-least-0.13 as a problem, as that'd likely be the case anyway
< wumpus>
it doesn't force actually using HD does it?
< sipa>
indeed
< MarcoFalke>
Also we don't do feature backports, so it should be easy
< wumpus>
right
< sipa>
wumpus: there is still time for release notes, right?
< jonasschnelli>
If we add another feature, we cannot go bellow FEATURE_HD. So all new features will require HD support.
< wumpus>
#8362 is lacking a rationale: what bug is fixed there?
< jonasschnelli>
We could no longer add features that run with <0.13
< wumpus>
sipa: yes, there is no need to do that before rc1
< wumpus>
jonasschnelli: they require hd *support*, they don't require the wallet to be hd
< wumpus>
jonasschnelli: the latter I would see as a problem, but not this
< jonasschnelli>
Sure. The point is only, that we can't add optional features that would run on 0.12 and below.
< MarcoFalke>
We don't do that?
< wumpus>
we could. Just don't increase the wallet min version?
< sipa>
i'd also like to see 8365 in... but it needs more review/discussion than we can do at this point, i'm afraid
< jonasschnelli>
wumpus: Yes. But you would need to increase it over FEATURE_HD.
< MarcoFalke>
I guess 8365 is not a blocker for rc1?
< wumpus>
jonasschnelli: why?
< wumpus>
you can give it any value, depending on the first version that would be able to cope with it. But aren't features always introduced in the future?
< sipa>
MarcoFalke: i think we've neglected fixing #8079 for some reason, and i don't like needing to say now "sorry, we didn't care enough to fix this bug we accidentally introduced"
< * jonasschnelli>
is caught in the corner
< jonasschnelli>
I think your right..
< jonasschnelli>
Yes. Its not a problem. Indeed.
< wumpus>
phew :)
< wumpus>
we shouldn't add any new blockers for rc1
< wumpus>
I'm sure rc2 will be necessary anyhow
< wumpus>
major releases always have long rc cycles
< wumpus>
the sooner we start testing, the better
< jonasschnelli>
Yes.
< MarcoFalke>
agree. There is so much to test
< wumpus>
unless there is a really a critical this-will-crash-everything or security issue, of course
< MarcoFalke>
Neither 8365 nor 8362 are critical
< sipa>
i think 8362 has actually 0 effect in practice
< sipa>
as GBT does not report the coinbase's sigops, and the 'generateblocks' call ignores it
< sipa>
src/leveldb in HEAD was last updated to upstream commit 20ca81f08fb7fa108923a091668e447dcf5c6b9d (tree 1c185cba2ddcb6a77ece1094bc22aea072db84d1)
< sipa>
src/leveldb in HEAD currently refers to tree 1c185cba2ddcb6a77ece1094bc22aea072db84d1
< sipa>
GOOD
< * sipa>
dusts off his gitian skills
< * MarcoFalke>
googles "release process bitcoin" and copies the commands
< sipa>
should i rebuild base image?
< MarcoFalke>
Gitian should handle this?
< jonasschnelli>
Not sure.
< jonasschnelli>
When did we update to Trusty? I guess sipa hasn't used gitian since the 0.12 release. :)
< sipa>
i have a trusty image already
< sipa>
but it's old
< MarcoFalke>
Mine is from Dec 13
< jonasschnelli>
Give it a try. :)
< sipa>
i assume there have been point releases?
< sipa>
already deleted the old one
< jonasschnelli>
Jan 6 2016 base-trusty-amd64.qcow2
< GitHub180>
[bitcoin] jonasschnelli opened pull request #8369: [FOR LATER USE][WIP][Wallet] add support for a flexible "set of features" (master...2016/07/wallet_features) https://github.com/bitcoin/bitcoin/pull/8369
< jonasschnelli>
sipa: I use KVM. Never got warm with LXC
< achow101>
yay gitian building
< sipa>
jonasschnelli: the gitian-building.md guide says use lxn
< sipa>
lxc
< jonasschnelli>
Its because wumpus uses LXC and write the doc. :)
< jonasschnelli>
*wrote
< sipa>
ah
< jonasschnelli>
But I think its good if we have build of both worlds.
< jonasschnelli>
*builds
< sipa>
so it should work and result in identical builds?
< jonasschnelli>
Yes. (I hope).
< jonasschnelli>
I always uses KVM and had the same hashes
< jonasschnelli>
MarcoFalke: what do I need to pass to gbuild to use parallelism? I now pass "-j2 --memory 3000" (had crashed with -j5)... but a tripple build takes >80mins.
< wumpus>
yes, KVM and LXC both work, and yield indentical results
< MarcoFalke>
j is default 2
< jonasschnelli>
Okay. I uses -j5 without -memory 3000 in the past... maybe this was the issue.
< jonasschnelli>
There are no other arguments?
< wumpus>
I use LXC because I don't like nesting qemus
< MarcoFalke>
but memory default is 2000, I think
< MarcoFalke>
./gbuild -help
< wumpus>
I have KVM on an older machine though and last time the gitian build just worked with it
< jonasschnelli>
We should really extend gitian-builder to support ECDSA and add a binary verified to Qt
< wumpus>
LXC is somewhat harder to set up though
< jonasschnelli>
*binary verifier
< MarcoFalke>
wumpus: Why nest. Run it on metal?
< jonasschnelli>
metal?
< MarcoFalke>
no virtual cpu
< MarcoFalke>
the actual hardware
< jonasschnelli>
ah
< wumpus>
MarcoFalke: meh, too much bother - the big machine i have runs tons of VMs, and I don't really want to install anything in the top level
< MarcoFalke>
I wonder how long it would take on my machine if I was using the nested approach
< wumpus>
(of course it would be possible to dual-boot it into something else, for example with an external harddisk, but that means shutting down all other things)
< sipa>
wumpus: you don't use VirtualBox?
< wumpus>
I use virsh (from libvirt), inside there I have a debian VM, and inside that runs LXC with the builder
< MarcoFalke>
But I always populate caches, so it runs quick on the actual tag
< wumpus>
virsh is a bit unwieldy to set up, but nice and efficient if you have lots of VM instances
< wumpus>
it doesn't have a nice user interface like virtualbox
< achow101>
what about vmware?
< jonasschnelli>
achow101: vmware, who knows whats running inside the VM then... :-)
< jonasschnelli>
(or on the host)
< jonasschnelli>
But I have to admit, I'm also using VMWare on my local mac. But for gitian it won't work I guess (at least not with significant fiddling).
< wumpus>
I don't know anything about vmware, last time I used it must be some time in the 90's :)
< wumpus>
but in principle you can run LXC inside *any* kind of virtualization
< sipa>
ah, kvm does not work inside virtualbox?
< wumpus>
as it simply uses linux' namespacing to keep processes contained, instead of virtualizing actual hardware, it doesn't need any special CPU bits either
< jonasschnelli>
sipa: no.
< jonasschnelli>
But in VM ware
< wumpus>
sipa: I don't think so; or only with special cpus
< sipa>
jonasschnelli: knowing that would have saved me a lot of time :)
< jonasschnelli>
sipa: AFAIK VirtualBox has no native KVM support
< jonasschnelli>
sipa: you didn't asked. :)
< wumpus>
there is something as 'nested virtualization' , but I have no idea what supports it
< sipa>
i was just following the guide, and you said KVM worked fine :)
< wumpus>
you can also disable the CPU virtualization completely and use x86 emulation, but then compiling will likely take a year or so :-)
< jonasschnelli>
sipa: I just run gitian-builder on my host
< jonasschnelli>
I don't care about running in host mode because it's in a remote servercenter anyways.
< jonasschnelli>
Also it copies around the build.log, etc.
< jonasschnelli>
achow101 looks pretty nice. You could also add support for building a PR and optional disable platforms...
< wumpus>
especially copying the result immediately after the build can avoid a lot of annoyance, is my experience
< achow101>
jonasschnelli: how does building from a PR work?
< wumpus>
my script does automatically copy files which should be uploaded (and rename where necessary), and creates a SHA256SUMS. Although this will need to be updated for 0.13, with the new naming for the linux files, thinking of it.
< jonasschnelli>
achow101: my scripts does something like: /bin/gbuild "+gbuildadd+" --commit signature="+VERSIONCOMMIT+"
< sipa>
wumpus: ah, yes, i'll need to update my script as well...
< wumpus>
the rest is pretty much just copy/paste from the release notes. So at some point the script needs to be customized that much before it's useful, that providing an example isn't much more useful than just pointing at the release notes :)
< wumpus>
yes --url bitcoin=${URI} --commit bitcoin=${COMMIT} can be very useful. You can also use local paths.
< achow101>
how big should I make -j and -m to make it build faster?
< sipa>
-j: not more than you have cpu cores
< sipa>
-m: something like 1500 times the number you pass to j
< sipa>
but -m also not more than the physical memory you have in your vm
< kinlo>
-j should be a bit higher then cores right? you usually end up with processes waiting for disk and not utilizing cpu, so oversubscribing a bit should increase speed
< sipa>
i've tried various numbers between 5 and 10 (i have 4 core, 8 threads), but it doesn't seem to matter much for overall compilation speed
< wumpus>
kinlo: I usually use cores+1 for that reason, but haven't benchmarked wether it is any faster
< sipa>
my gitian build scripts seems to use -j 4 -m 8000
< sipa>
but no idea how those numbers came about
< wumpus>
compiling bitcoin is quite CPU intensive, so the i/o overhead may be very little in comparison, it may not be the same for every project
< wumpus>
e.g. for a project such as tor with tons of C files that compile very quickly, I think using -j >> cores will achieve more, as all those gcc's spend a relatively large part of their lifespan accessing disk
< kinlo>
I usually only compile the kernel, noticed speedups there but indeed, makes sense...
< wumpus>
kernel is similar
< achow101>
I set my -j to double the cores I have. I don't know whether that makes a significant difference though
< cfields>
wumpus / jonasschnelli / achow101: could one of you make your osx build results available for comparison? I'm building now, not sure exactly what to ask for in particular yet.
< cfields>
jonasschnelli: hmm, our end-result osx binaries are the same, despite the different tarball hashes. I'm thinking it might just be timestamps in .tar's or so
< cfields>
looking deeper now.
< cfields>
whoops, nm. the osx64.tar.gz is the same for everyone.
< michagogo>
Wait, we bumped the SDK?
< MarcoFalke>
jup
< michagogo>
:-/
< * michagogo>
looks up details
< cfields>
achow101 / jonasschnelli: either of you have the bitcoin-0.13.0-osx-unsigned.tar.gz available?
< achow101>
cfields: is that file supposed to be produced? I don't see it
< cfields>
achow101: your script probably automatically moves it to gitian/inputs
< michagogo>
(I'm still not used to having things like my browser launch instantaneously...)
< achow101>
cfields: found it (I think) and uploaded to the page I linked earlier
< cfields>
achow101: thanks
< jl2012>
to build for osx, should I just put the MacOSX10.11.sdk.tar.gz to the inputs directory?
< achow101>
jl2012: yes
< jl2012>
thanks
< sdaftuar>
sipa: in #8365 i noticed you didn't change the rpc handlers to output the sigops-adjusted-vsize. was that intentional? looks like the mempool rpc's would do the right thing, but decoderawtransaction would report unadjusted vsize
< cfields>
aha, found the difference
< cfields>
looks like the DS_Store generator isn't deterministic somehow. mine doesn't match achow101's
< sipa>
sdaftuar: it's intentional, but i'm not sure what the best approach is
< MarcoFalke>
You can also run twice and it doesn't match
< achow101>
cfields: so how is that fixed?
< cfields>
achow101: looking now to see how/why it differs
< sdaftuar>
sipa: seems like the cleanest thing would be for vsize to our policy-adjusted vsize, and for us to add a field to the output which is the transaction weight
< sipa>
sdaftuar: the difficulty is determining the sigops...
< sipa>
for wallet txn
< sipa>
for raw transactions it's just impossuble
< sdaftuar>
i was thinking that there's an ambiguity for things that are in our mempool, but right i see your point
< sipa>
i'd rather try to make sure we never produce transactions ourselves for which it has an effect :)
< sdaftuar>
yeah, that might work fine for the wallet code. i'm a little worried that the approach of having two different sizes we use, a policy size and an actual size, will be error prone in the future.
< sdaftuar>
so i was just trying to go through and identify every usage to make sure it made sense
< sdaftuar>
seems like if we do end up eventually changing bytespersigop to 50, it'll be easy for wallet users to accidentally generate transactions that are affected by the policy
< sdaftuar>
if you have a bunch of p2pkh outputs, you'll hit it
< sipa>
really?
< sipa>
signatures are 72 bytes
< sdaftuar>
outputs -- legacy sigop count will get you
< sdaftuar>
that's the one i found with sigop cost of 9644
< BlueMatt>
wumpus: needed a fix for "I think IsKeyType should be changed to include hdchain, not add an explicit hdchain check in ::Recover, since we probably definitely want it in ::LoadWallet as well." first
< BlueMatt>
that is very bad behavior
< BlueMatt>
oh, you already tagged rc1...ugh
< BlueMatt>
wumpus: also you merged 8367 without fixing the bug I pointed out in it..........
< gmaxwell>
BlueMatt: open a PR to fix it. your "not that it really matters" might have understated your expectation that it would get fixed.
< BlueMatt>
I mean I figured it'd get fixed before merge, even if its not a huge deal
< sipa>
BlueMatt: i'm sure there'll be a second rc anyway
< sipa>
i guess wumpus wanted to get an rc out to get testing from a wider audience
< gmaxwell>
we're seriously late on the RC. It was good to get one out.
< cfields>
i think i have the osx determinism issue worked out. any gitian builders around to verify?
< BlueMatt>
yeayea, I'm not suggesting anything is gonna catch fire, just saying there are a few things here to fix for rc2
< MarcoFalke>
The check is dead code and the strings in the exceptions need the function names adjusted ...
< MarcoFalke>
It is good to have rc1 out. We can fix those later
< MarcoFalke>
cfields: I can try
< cfields>
MarcoFalke: ok. tested by hand, plugging it all in now. ready in ~10min
< MarcoFalke>
Is it just a descriptor bump or do we need rc2 already?
< achow101>
cfields: I can try it too
< cfields>
MarcoFalke: i'll leave it up to wumpus. it's the .DS_Store, which is inconsequential and has no influence on the binary. but it means that gitian results won't match.
< michagogo>
Got the new SDK -- my rc1 is building right now
< luke-jr>
did we bump ds_store Python or smth? O.o
< cfields>
luke-jr: think there's any way to post-process an existing ds_store with unsorted plists inside so that we can just fix it up in the gitian descriptor for rc1?
< luke-jr>
I would imagine so, but meh? :/
< cfields>
luke-jr: otherwise i don't think i can sign rc1
< luke-jr>
cfields: can DS_Store do anything malicious? why not just compare the binaries without it?
< cfields>
hmm, i guess that's not right. that file's outside of the .app, so i suppose it's not signed at all
< luke-jr>
right
< luke-jr>
so it's not a technical issue AFAIK, just potentially philosophical
< cfields>
yep. i guess a few builders could manually fixup inside gitian to force an order, then at least there's a reference for a matching hash
< cfields>
but yes, since it's not signed data and everyone's binaries should match, i'll carry on signing as usual
< michagogo>
If you push the sigs before my script finishes running, it should grab them and generate sigs for the -signeds too
< michagogo>
(I'm going to sleep now, but the PR should be pushed automatically)
< MarcoFalke>
achow101: You applied the patch on the gitian vm?
< achow101>
yes
< MarcoFalke>
cfields: Let us know if signing the binaries worked out for you
< * luke-jr>
ponders how to best get P2SH^2 deployed.
< luke-jr>
at least bloated spam discourages miners from including it
< cfields>
MarcoFalke: will do, still working on it.
< MarcoFalke>
great
< achow101>
ran the build again and got the same results
< cfields>
achow101: to clarify, you're testing master now, right?
< achow101>
yes
< cfields>
ok
< cfields>
0.13.0rc1 sigs pushed.
< cfields>
achow101: building master with that PR on top now. thanks for doing that so quickly :)
< cfields>
achow101: actually, before i build, could you specify the commit you built? Not sure if you cherry-picked or merged, and that'll change the result
< achow101>
I grabbed the diff and applied it.
< achow101>
probably wasn't the right thing to do, but i was being lazy
< cfields>
achow101: mm, i don't think i'm going to be able to match your result, then
< achow101>
err. actually I think I just built your branch. I was originally just going to use the diff but I screwed something up
< cfields>
3b3ce25df6cc, then?
< achow101>
yes
< cfields>
ok great, building that.
< phantomcircuit>
jonasschnelli, i rebased 8152 can you review again
< phantomcircuit>
jonasschnelli, why does CHDKeyStore::PrivateKeyDerivation take the keypath as a string?
< phantomcircuit>
sipa, either way it's missing ntweak entirely
< sipa>
ah
< sipa>
eh, no?
< phantomcircuit>
oh no it's not
< phantomcircuit>
what
< Chris_Stewart_5>
nTweak is there, is there suppose to be a tweak along with it?
< phantomcircuit>
oh the weird warning makes it look like there's nothing below the fold
< sipa>
Chris_Stewart_5: to answer your first question: yes
< phantomcircuit>
yeah nvm it's right
< sipa>
it's a compact integer
< Chris_Stewart_5>
I've had issues even trying to find the classes/structs that correspond to some of these messages, where can I find the actual message structure in core?
< Chris_Stewart_5>
If I do a search I find references to 'FilterLoad' for instance in main.cpp, protocol.cpp and protocol.h but no class or struct, unless i'm missing something obvious
< sipa>
else if (strCommand == NetMsgType::FILTERLOAD)
< sipa>
{
< sipa>
CBloomFilter filter;
< sipa>
vRecv >> filter;
< sipa>
CBloomFilter is defined in bloom.h
< Chris_Stewart_5>
and there is some ffunction defined in CBloomFilter to deserialize the DataStream?
< Chris_Stewart_5>
I'm just following the stack and it seems like we go from streams.h -> serialize.h which starts parsing the varint, but I don't see where the contents are explicitly passed to a constructure or anything
< Chris_Stewart_5>
is there some C++ convention going on here?
< sipa>
there is a lot of magic in serialize.h
< sipa>
ADD_SERIALIZE_METHODS expands into the necessary serialization boilerplate
< Chris_Stewart_5>
oh ok. Does it use some sort of reflection to call the necessary constructors?
< sipa>
there are no constructors inbolved
< sipa>
the constructor of CBloomFilter is invoked on the line where filter is declared
< sipa>
the 'vRecv >> filter' just reads data from vRecv stream and fills filter with it
< Chris_Stewart_5>
and this 'fill' function is defined in ADD_SERIALIZE_METHODS?
< GitHub8>
[bitcoin] pstratem opened pull request #8375: Move key derivation logic from GenerateNewKey to DeriveNewKey (master...2016-07-19-cwallet-derivenewkey) https://github.com/bitcoin/bitcoin/pull/8375
< sipa>
yes, it's called Deserializr
< sipa>
Deserializr
< sipa>
grr, stupid keyboard
< sipa>
Deserialize
< Chris_Stewart_5>
Your 'e' key wants you to go to sleep :-). Thanks sipa
< phantomcircuit>
i think it's easier to read that way ^